Skip to content
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

Set default project for WorkerJob #645

Merged
merged 3 commits into from
Jan 24, 2022
Merged

Set default project for WorkerJob #645

merged 3 commits into from
Jan 24, 2022

Conversation

pmrv
Copy link
Contributor

@pmrv pmrv commented Jan 20, 2022

The WorkerJob needs an input project assigned, but the default is None and it's not checked before the worker job starts running, which leads to silent death of the worker if you forget to assign one. I've set the default now to be the project of the worker itself, because that seems to me the common use case. If that's too specific we should instead add a validate_ready_to_run method that does this check.

@pmrv pmrv added the bug Something isn't working label Jan 20, 2022
@pmrv pmrv requested a review from jan-janssen January 20, 2022 15:56
@pmrv
Copy link
Contributor Author

pmrv commented Jan 20, 2022

Mh, I've noticed now that this creates the problem that the worker is waiting for itself to finish, which is also a bit borked...

@jan-janssen
Copy link
Member

That is strange - I initially had an implementation where the worker was watching a specific project, but by now it should only filter based on the job_id. So the worker should not wait for itself.

@pmrv
Copy link
Contributor Author

pmrv commented Jan 21, 2022

That is strange - I initially had an implementation where the worker was watching a specific project, but by now it should only filter based on the job_id. So the worker should not wait for itself.

Ah, I think I just got confused by the worker log. I had tried to create & submit my worker job, submit all the smaller calculations and then immediately set the worker status to collect, hoping that this would shut down the worker once all the jobs are finished. Alas, when the worker started running on the node it changed its status away from collect to running and therefore did not stop once it was done. This is a little bit of a pain point, but I'll make a separate issue for this + some refactors.

The new default works for me, if you don't have objections I'll merge this later today.

@jan-janssen
Copy link
Member

I guess what you are looking for is the wait_for_worker() function. It stops the worker once all calculation are finished and no new jobs are submitted plus the timeout.

Copy link
Member

@jan-janssen jan-janssen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

Co-authored-by: Jan Janssen <[email protected]>
@pmrv
Copy link
Contributor Author

pmrv commented Jan 22, 2022

I guess what you are looking for is the wait_for_worker() function. It stops the worker once all calculation are finished and no new jobs are submitted plus the timeout.

The issue was that I didn't want to block the notebook, but continue working on other things, but I haven't come up with anything better yet.

@pmrv pmrv merged commit f6989a9 into master Jan 24, 2022
@delete-merged-branch delete-merged-branch bot deleted the workerproject branch January 24, 2022 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants