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

Extension of the Worker Class #575

Merged
merged 8 commits into from Dec 13, 2021
Merged

Extension of the Worker Class #575

merged 8 commits into from Dec 13, 2021

Conversation

jan-janssen
Copy link
Member

No description provided.

Copy link
Member

@liamhuber liamhuber left a comment

Choose a reason for hiding this comment

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

Would be nice to offer some flexibility in how patient the user wants to be

pyiron_base/job/worker.py Outdated Show resolved Hide resolved
pyiron_base/job/worker.py Outdated Show resolved Hide resolved
pyiron_base/job/worker.py Outdated Show resolved Hide resolved
@pmrv
Copy link
Contributor

pmrv commented Dec 9, 2021

Could this be implemented in terms of Project.wait_for_jobs? If not then the arguments @liamhuber suggested should match that function. I'm fine with having this extra function for convenience, but we should avoid duplicated logic or interfaces.

@jan-janssen
Copy link
Member Author

Could this be implemented in terms of Project.wait_for_jobs? If not then the arguments @liamhuber suggested should match that function. I'm fine with having this extra function for convenience, but we should avoid duplicated logic or interfaces.

The difference is basically that it searches for jobs based on the master_id rather than the path, which allows you to have multiple workers inside the same project and it sets the status of the worker to collect once all jobs are finished to stop the worker. So while I foresee in the long run the worker jobs just like pyirontable being meta jobs the user never actively works with, at the current stage I prefer to have a separate function.

@liamhuber
Copy link
Member

I see the merit of having such wait code directly on the worker master, but Marvin's point about not duplicating things is also a very good one. Can we have our cake and eat it too?

What if we extract the waiting code, such that the source, times, etc. are all taken as arguments, then exploit this extracted behaviour at both the worker and project levels. The additional behaviour that the worker sets itself to collect should be easy enough to append after.

I'm sitting at 30k ft on this, so forgive me if I miss some impossible architecture obstacle. Seems like it should work though..

@jan-janssen
Copy link
Member Author

I'm sitting at 30k ft on this, so forgive me if I miss some impossible architecture obstacle. Seems like it should work though..

While the functions have a similar effect for the user, their technical implementation is different e.g. filter by master id, write log file and only finish after time out, for the case the tasks for the worker are submitted from a different notebook. So I currently do not see an option to combine them. Internally they both use the job_table() to get the data from the database but the filtering and the reaction are different.

@jan-janssen jan-janssen merged commit 31887ae into master Dec 13, 2021
@delete-merged-branch delete-merged-branch bot deleted the wait-worker branch December 13, 2021 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants