Improve error handling and helpers for failed tasks #415
Conversation
Signed-off-by: Aurélien Tamisier <[email protected]>
ybastide
force-pushed
the
improvement/DATA-16022/Simpleflow-Improve-error-handling-and-helpers-for-failed-tasks
branch
2 times, most recently
from
bb14952 to
8da51c5
Compare
Signed-off-by: Yves Bastide <[email protected]>
Signed-off-by: Yves Bastide <[email protected]>
Signed-off-by: Yves Bastide <[email protected]>
* create task_error, task_error_type on init * add reason, details Signed-off-by: Yves Bastide <[email protected]>
Signed-off-by: Yves Bastide <[email protected]>
Signed-off-by: Yves Bastide <[email protected]>
ybastide
force-pushed
the
improvement/DATA-16022/Simpleflow-Improve-error-handling-and-helpers-for-failed-tasks
branch
from
8da51c5 to
9fff72d
Compare
| logger.info( | ||
| "spawning new activity id=%s worker pid=%d heartbeat=%s", | ||
| task.activity_id, | ||
| os.getpid(), |
There was a problem hiding this comment.
I believe displaying the pid should be the role of the log formatter, not the log message itself. https://docs.python.org/3/library/logging.html#logrecord-attributes
There was a problem hiding this comment.
... and it is. Good catch!
There was a problem hiding this comment.
Not sure how to handle this though, as we integrate Simpleflow in a Django project, and inherit from the logger configured in the project's settings 🤔
There was a problem hiding this comment.
Maybe add "formatter": "simpleflow_formatter", to the Django project's settings? What could go wrong? 🙃
EDIT: simpleflow does use its logger, so the pid is present
There was a problem hiding this comment.
Double-checked:
- we do not override Simpleflow's logging options in our workers
loggingis initialized withsetup_logging()at https://github.com/botify-labs/simpleflow/blob/main/simpleflow/log.py#L148 , that relies on the defaultLOGGINGdict at https://github.com/botify-labs/simpleflow/blob/main/simpleflow/settings/default.py#L27- this sets up
SimpleflowFormatterhttps://github.com/botify-labs/simpleflow/blob/main/simpleflow/log.py#L49 as the default formatter - workers use
loggersobtained fromlogging.getLogger()
What I'm not clear on though, is whether we use:
- the format string set in https://github.com/botify-labs/simpleflow/blob/main/simpleflow/settings/default.py#L50
- the format string set in https://github.com/botify-labs/simpleflow/blob/main/simpleflow/log.py#L87
There was a problem hiding this comment.
Right, it's confusing...
simpleflow worker.start -N1 --task-list example
2023-07-18T16:11:48 INFO [process=MainProcess, pid=34924]: starting <bound method Poller.start of ActivityPoller(domain=TestDomain, task_list=example)>
Same on Papertrail
Already present in the default log formatter. Signed-off-by: Yves Bastide <[email protected]>
I'm not sure how to do it cleanly, so... Signed-off-by: Yves Bastide <[email protected]>
right, I'll fill the descrition 🙂 |
Signed-off-by: Yves Bastide <[email protected]>
This should not be necessary anymore... Signed-off-by: Yves Bastide <[email protected]>
|
Also, 🧇 ! |
ybastide
deleted the
improvement/DATA-16022/Simpleflow-Improve-error-handling-and-helpers-for-failed-tasks
branch
Implement #416.
@virtualtam would this match what you want? What's missing?