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

Allow list of jobs from scheduler using username #187

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

gpetretto
Copy link
Contributor

Some schedulers (e.g. SGE) do not allow fetching job details providing a list of job ids, which is how jobflow-remote usually gets information about the running jobs. See Matgenix/qtoolkit#43. To avoid this issue an option to use the username as filtering option has been added. If it is present, the runner will use that instead of the list of ids.

@QuantumChemist, do you think this will be fine for your use case?

@codecov-commenter
Copy link

codecov-commenter commented Sep 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.92%. Comparing base (1b97f4c) to head (e5e922f).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #187      +/-   ##
===========================================
+ Coverage    47.89%   47.92%   +0.03%     
===========================================
  Files           43       43              
  Lines         5153     5156       +3     
  Branches      1117     1118       +1     
===========================================
+ Hits          2468     2471       +3     
  Misses        2425     2425              
  Partials       260      260              
Files with missing lines Coverage Δ
src/jobflow_remote/config/base.py 82.64% <100.00%> (+0.07%) ⬆️
src/jobflow_remote/jobs/runner.py 65.96% <100.00%> (ø)
src/jobflow_remote/remote/queue.py 66.01% <100.00%> (+0.67%) ⬆️

@davidwaroquiers
Copy link
Member

Thanks @gpetretto

This option could also enable a "global" max number of jobs per user on a worker right ? (we already talked about such an option as it is sometimes a constraint in some supercomputers).

@QuantumChemist
Copy link
Contributor

This sounds very good to me. I guess we simply have to test it how (in)convenient it will be in the end and take adjustments accordingly 😃

@QuantumChemist
Copy link
Contributor

Thanks @gpetretto

This option could also enable a "global" max number of jobs per user on a worker right ? (we already talked about such an option as it is sometimes a constraint in some supercomputers).

that would be very useful for our clusters (that run with slurm) as well

@gpetretto
Copy link
Contributor Author

This option could also enable a "global" max number of jobs per user on a worker right ? (we already talked about such an option as it is sometimes a constraint in some supercomputers).

Indeed it could be a step in that direction. However, the logic of the Runner is still to check only the processes corresponding to that worker, while the count of processes running on a specific runner comes from the DB. So more changes and possibly more options in the configuration will be needed to provide that functionality.

@davidwaroquiers
Copy link
Member

Sure, I wasn't suggesting to do it now but basically, we need this option before being able to possibly think about implementing/adding this feature.

Copy link
Member

@davidwaroquiers davidwaroquiers left a comment

Choose a reason for hiding this comment

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

Thanks, Good to me, just one comment/question

src/jobflow_remote/config/base.py Show resolved Hide resolved
@QuantumChemist
Copy link
Contributor

QuantumChemist commented Sep 20, 2024

My colleague could with a lot of experimenting now figure out how you can actually do query via job list

qstat -j 203614,203613,203612

Screenshot 2024-09-20 at 11 48 07

Sorry for not finding this out earlier. But I still find querying via username very useful (also for my own use cases with slurm)

I will also adjust this in the qtoolkit PR!

@gpetretto gpetretto merged commit 6281728 into develop Sep 25, 2024
5 checks passed
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.

4 participants