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

Rework Job Retrieval/Submission API #224

Closed
20 tasks done
tazend opened this issue Dec 13, 2021 · 13 comments · Fixed by #283
Closed
20 tasks done

Rework Job Retrieval/Submission API #224

tazend opened this issue Dec 13, 2021 · 13 comments · Fixed by #283
Assignees

Comments

@tazend
Copy link
Member

tazend commented Dec 13, 2021

Branch is here

Classes

  • Job class providing read-only access to all possible attributes of jobs (e.g. time of submission)
  • Jobs class, acting as a custom collection (dict) to retrieve actual job-information, and holds all the Job instances which carry the information.
  • JobStep class providing read-only access to all possible attributes of a single Job-step
  • JobSteps class, acting as custom collection (dict) to hold all the 'JobStep' instances
  • JobSubmission class, acting as a interface to create a job-submission records and to submit them

General Functionality for a Job

  • cancel (SIGKILL)
  • send_signal (e.g send any signal like SIGTERM,SIGKILL,SIGUSR1 to the job)
  • suspend/unsuspend
  • hold/release
  • modify (user must pass a JobSubmission instance with the changes to be made)
  • notify
  • Retrieve the batch-script and return it as a string

General Functionality for a JobStep

  • cancel
  • send_signal
  • modify (only changing the TimeLimit is allowed for a job step

Other

  • Documentation
  • Write tests
  • Write examples

Related to #11

@tazend tazend self-assigned this Dec 16, 2021
@tazend
Copy link
Member Author

tazend commented Dec 16, 2021

Hi @giovtorres , I think I'm making some good progress with the job class(es) (job-submission, retrieval, etc..). Would it be possible to maybe create a seperate branch for this refactoring process? Maybe something like dev-refactor or just dev branch , where I could push the progress? (I think I'm not able to create branches here). I'd really like to bring the project forward in this regard.

What do you think?

@giovtorres
Copy link
Member

Great to hear the progress! Go ahead and create a branch. You should be able to now.

@tazend
Copy link
Member Author

tazend commented Mar 29, 2022

I will continue to work on this again now.

Haven't done anything on this in a while, however last time I already got pretty far with the rework (except a few job-submission parameters that require a bit more extensive input-validation.)

@tazend tazend changed the title Refactor: Job class(es) Rework Job Retrieval/Submission API Apr 24, 2022
@tazend
Copy link
Member Author

tazend commented Apr 24, 2022

Made a new branch here with current progress.

@tazend
Copy link
Member Author

tazend commented Oct 4, 2022

Pretty much 90% of the general functionality is now implemented/reworked, though it will still need some bug-fixing probably, polish and fixing some outstanding TODO's. And need to write tests for this.

@multimeric
Copy link
Contributor

This change looks good (main...rework/jobs). This is the object oriented wrapper I was hoping pyslurm would have. Do you need any help getting this finished, e.g. code review?

The main thing I'm after is actually a wrapper around slurmdb_jobs, which you haven't covered in this PR. Is that something you've started elsewhere? If not, I can give it a go. Would it make more sense to branch off this (very large) branch, or main?

@tazend
Copy link
Member Author

tazend commented Mar 1, 2023

Hi @multimeric

Good timing :)
I do actually have something in the works already for a wrapper around the database Job API, and it is the next thing that I was going to put my focus on, as its also a part of Slurm that users interact with quite often.
I haven't pushed it yet, but I'll push a new branch based on this one and try to get it up later today when I'm back from work. I'll let you know and then I'd definitely appreciate feedback on it :)

For the rework regarding this issue - its pretty much done. Was just going to check for some last possible changes and some missing documentation/examples. But I do definitely want to get it finally merged and available for people in the next few days, and improvements/missing features can be applied continuously.

@multimeric
Copy link
Contributor

Wonderful, let me know when you need help!

@tazend
Copy link
Member Author

tazend commented Mar 9, 2023

Functionality wise I consider this one here now done and would focus on getting this merged.
The only thing I'm going to do later today is fixing the docstrings for all the properties, so mkdocs is able to generate the documentation correctly.

Since I've used google docstring style exclusively here (which I find better than ReST), the docstrings in pyslurm.pyx will also need to be converted to this style.

The plan is to have this sort of New API live side by side with the old stuff in pyslurm.pyx for some time, and declare any old class in pyslurm.pyx as deprecated if there is a replacement (e.g. the job class in pyslurm.pyx is completely replaced now by this new Job class now)

What needs to be done is laying out the documentation for this correctly, having one reference API for the new stuff, and the reference API for the "old" stuff in pyslurm.pyx (#271)
Documentation wise each Class should ideally have its one page, as classes have lots of attributes and will be pretty big.
In the future it would probably also be benefitial to include specific examples on how to use thing, in addition to the reference API.

@multimeric
Copy link
Contributor

Okay, sounds good. Let me know if you would like any help with anything.

@tazend
Copy link
Member Author

tazend commented Mar 9, 2023

Sure, if you'd like to I would definitely appreciate help with the documentation stuff, somewhat in the direction like I described. (or of course if you have better ideas to properly structure both reference APIs, let me know :) )

I'd suggest that I merge #271 already as it is basically functioning and transitions the current documentation as is to mkdocs. Based on that you could then do the necessary steps in a clone of the branch regarding this issue to:

  • structure documentation properly for both new and old API
  • fix the docstrings on attributes so they work with mkdocs (like I started here
  • transition pyslurm.pyx to google style docstrings.

Would that be fine?

@multimeric
Copy link
Contributor

Is this planned to be included in a new release? Which version(s) of Slurm will it be compatible with?

@tazend
Copy link
Member Author

tazend commented May 9, 2023

@multimeric

yes, I will tag a new release soon.
Then I will also go ahead and try to backport this to older versions, perhaps as far back as 20.02 or 20.11. Nothing before 20.02 though for now, just to draw the line somewhere, since these versions are pretty old.

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 a pull request may close this issue.

3 participants