-
-
Notifications
You must be signed in to change notification settings - Fork 750
Don't invoke log_event from state machine #6512
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
Conversation
Unit Test Results 15 files ±0 15 suites ±0 6h 33m 47s ⏱️ + 2m 53s For more details on these failures, see this check. Results for commit ef12618. ± Comparison against base commit bd98e66. ♻️ This comment has been updated with latest results. |
|
|
||
| del s.events["invalid-worker-transition"] # for test cleanup | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicates test in test_utils_test.py
|
Ready for final review and merge |
| if isinstance(msg, dict): | ||
| msg["worker"] = worker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious, why this change?
For context, this tripped me up when doing some Coiled work. I'm not against it, I'm just curious about the motivation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, the same info was manually added to most worker-side calls to log-event. Adding it when it lands looks cleaner and less redundant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users can use this system too though. We're adding this to their messages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, when do you sleep? I'm about to finish up my work day in California. If you're working past when I do then that really puts me to shame :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(travelling away from home in Texas this week)
Worker._transitionWorker.handle_stimulusandWorker.validate_state; they will remain in Worker post refactor, in methods that override those inBaseWorker.