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

python: promote JobInfo class to bindings #3140

Closed
grondo opened this issue Aug 13, 2020 · 3 comments
Closed

python: promote JobInfo class to bindings #3140

grondo opened this issue Aug 13, 2020 · 3 comments
Assignees

Comments

@grondo
Copy link
Contributor

grondo commented Aug 13, 2020

As a first step toward #3032, lets at least uplift the JobInfo class into the Python bindings.

Requiring users of the job_list() API to decode some of the low level data in the response seems unkind (e.g. state and result are just integers). By allowing wider use of JobInfo class we can make the interface much more usable.

Perhaps a JobInfo constructor could also be used in place of, or as an enhancement to, the custom JobRecord class discussed in #3136.

@grondo
Copy link
Contributor Author

grondo commented Aug 22, 2020

Just looking into doing a quick experiment moving the JobInfo class into the flux.job module, however this will start to make job.py pretty long.

I don't have much experience with Python module development, can we easily split job.py into multiple files by making a new directory src/bindings/python/flux/job with an __init__.py, and split existing code where it makes sense (e.g. jobspec.py, list.py, etc.)? Or will that break the way flux.job methods are imported?

@SteVwonder
Copy link
Member

I don't have much experience with Python module development, can we easily split job.py into multiple files by making a new directory src/bindings/python/flux/job with an init.py, and split existing code where it makes sense (e.g. jobspec.py, list.py, etc.)? Or will that break the way flux.job methods are imported?

Yep. You can do that. To preserve backwards-compatibility, I think we will need to add something like this to the job/__init__.py to pull all of the definitions into the flux.job "namespace":

from flux.job.jobspec import Jobspec, JobspecV1
from flux.job.id import id_parse, id_encode, JobID
from flux.job.control import *

where jobspec, id, and control are just strawman names for this example (I'm not certain that id will even work given that Python defines a bulit-in id).

@grondo
Copy link
Contributor Author

grondo commented Aug 23, 2020

Thanks! That was the hint I needed...

@grondo grondo added this to the flux-core 0.19.0 milestone Aug 28, 2020
@grondo grondo self-assigned this Aug 28, 2020
grondo added a commit to grondo/flux-core that referenced this issue Aug 28, 2020
Problem: The JobInfo class in flux-jobs is useful for abstracting the
raw job data returned from the job-info list APIs into a more usable
interface, but it is not available to Python bindings users.

Promote the JobInfo class to flux.job, so it may be used by other
Flux components and users of the Python API.

Fixes flux-framework#3140
@mergify mergify bot closed this as completed in a2bd948 Aug 31, 2020
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

No branches or pull requests

2 participants