-
Notifications
You must be signed in to change notification settings - Fork 18
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
Spine Toolbox changes related to ditching Dagster #2715
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 0.8-dev #2715 +/- ##
========================================
Coverage 65.75% 65.75%
========================================
Files 196 196
Lines 35869 35870 +1
Branches 6094 6094
========================================
+ Hits 23584 23586 +2
+ Misses 11023 11021 -2
- Partials 1262 1263 +1 ☔ View full report in Codecov by Sentry. |
After a small edit, I'm not finding any problems here anymore. If anybody wants to check this out, it would be much appreciated. No need to check code if you're too busy, just execute a couple of projects and see if something doesn't look right. See install instructions above. |
Stopping a Tool (tested with SpineOpt) does not seem to work. The hammer just keeps hammering and when I quit Toolbox, there is a rogue process that prevents Toolbox from shutting down properly. |
Besides the above problem with stopping a workflow, I did not find anything of note. Nice work @manuelma and @PekkaSavolainen! |
Does this work a lot better with dagster? |
Yes. I didn't run into similar problem with |
Stopping execution depends on luck on 0.8-dev as well. Other times it works well but I just got this traceback after stopping SpineOpt in 0.8-dev branch.
And there was a rogue python process after trying to quit toolbox that I needed to end in the task manager to actually close the app. |
Ok I see the problem, Clicking Stop in the |
We got the hammer problem fixed with @PekkaSavolainen. I believe we are now on par with the previous Dagster implementation. |
I'll update the changelog and run black. Do you mind if I merge when I'm done? |
I suppose this is all the testing we're ever getting from this branch, so no, I do not mind. |
As per title.
To test Spine Toolbox without dagster, checkout branch
issue_2523
, make a fresh conda env and dopip install -r requirements.txt
.Re Issue #2523
Checklist before merging