Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementing task desc croping(cf #27) #95

Closed
wants to merge 5 commits into from

Conversation

Skrattoune
Copy link
Contributor

Coming back to #27,
I had in a middle of a long series of tasks one of them which crashed due to a ValidationError('Process info description overlong').

It reminded me about #27 where I thought we implemented a crop of TaskModel.desc to 64 characters.

As stated already at the time, having a desc bigger than 64 char should not for me prevent a task to run.
Especially when task description are generated automatically

I would therefore suggest implementing the following changes

Coming back to boxine#27,
I had in a middle of a long series of tasks one of them which crashed due to a ValidationError('Process info description overlong').

It reminded me about boxine#27 where I thought we implemented a crop of TaskModel.desc to 64 characters.

As stated already at the time. having a desc bigger than 64 char should not for me prevent a task to run.
Especially when task description are generated automatically

I would therefore suggest implementing the following changes
@Skrattoune
Copy link
Contributor Author

Jens, note that that could lead to a fail of the test you put in place for testing if an error was raised when desc was bigger than 64

However, I don't really understand why an error would be raised on:
self.desc = self.desc[:TASK_MODEL_DESC_MAX_LENGTH]

@Skrattoune
Copy link
Contributor Author

Unclear for me why we should have the error instance = <[TypeError("unsupported operand type(s) for -: 'datetime.datetime' and 'NoneType'") raised in repr()] TaskModel object at 0x7f47379f1310> while running the testing.

Your help is needed

@Skrattoune
Copy link
Contributor Author

Need your input @jedie

Copy link
Contributor Author

@Skrattoune Skrattoune left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated to match your suggestion

huey_monitor/tqdm.py Show resolved Hide resolved
jedie pushed a commit that referenced this pull request Aug 2, 2022
Adopt the idea from #95 and just crop the
description and log a warning.
jedie pushed a commit that referenced this pull request Aug 2, 2022
Adopt the idea from #95 and just crop the
description and log a warning.
@jedie
Copy link
Collaborator

jedie commented Aug 2, 2022

@Skrattoune i used our idea and create: #108

@jedie jedie closed this Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants