-
Notifications
You must be signed in to change notification settings - Fork 81
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
Fix multiple validation jobs #3465
Conversation
…bs, but the later must also made known by the DB
Got it, thank you for the fix. Do you think it will be possible to add a retroactive test that brakes without the fix and passes now with it? |
We would need to have a plugin that generates multiple artifacts as a test case. Adding the environment, registering all plugins to be "active", executing the workflow up to this point runs approx 15min on my laptop. Not sure if we want to invest that much compute in github actions. |
That's right and after doing a quick search in the code base I think this is happening in |
I think the problem with the existing test is that it manually sets the state of the dependent job here to "running": qiita/qiita_db/test/test_processing_job.py Line 347 in 5ab3ebb
|
The existing test does not seem to actually execute jobs. Creation of the biom files is faked here: qiita/qiita_db/test/test_processing_job.py Lines 309 to 319 in 5ab3ebb
Since the submit() function is creating true child processes, I am not sure if we really want to invest in writing a mock to test this behaviour. Maybe an alternative is to just test the DB fingerprint of what a submit would change?! But than it is not truly testing submit . I don't see an easy way forward :-/
|
OK, thank you for checking. I agree with your assessment. |
Addressing #3379:
With the proposed changes, both validation jobs of a deblur command successfully run through in my local setup. I suspect two mistakes of the current code:
qiita/qiita_db/processing_job.py
Lines 1091 to 1114 in 5ab3ebb