-
Notifications
You must be signed in to change notification settings - Fork 5
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
Remove parameter caching inside task class #370
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #370 +/- ##
==========================================
+ Coverage 88.70% 88.89% +0.19%
==========================================
Files 43 43
Lines 1797 1792 -5
==========================================
- Hits 1594 1593 -1
+ Misses 203 199 -4 ☔ View full report in Codecov by Sentry. |
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.
This change look good, thanks!
My only note is that the new tests currently pass on main
even without the associated changes to task/reworker.
This is because test_rest_api.py
is only using the "internal" non-subprocess handler, and it highlights that we probably need to either convert it (relates partially to #360), or introduce some system tests.
Yeah I think #360 should go towards solving that, it's useful to have the test regardless |
Closes #369 The validated and processed parameters from a plan request were being cached inside the `Task` class because they were needed in two separate threads. Unfortunately, with the introduction of the subprocess (#343), pickling was causing issues with the cached object (see #369 for more details). This PR is the simplest solution: remove the cache and generate the parameters twice. It also adds regression tests for the bug case in #369
Closes #369
The validated and processed parameters from a plan request were being cached inside the
Task
class because they were needed in two separate threads. Unfortunately, with the introduction of the subprocess (#343), pickling was causing issues with the cached object (see #369 for more details).This PR is the simplest solution: remove the cache and generate the parameters twice. It also adds regression tests for the bug case in #369