Skip to content

Conversation

@ahmedhany93
Copy link

We have edited pyslurm.pyx file to fix the ntasks job option which was commented out, fix the work_dir job option which was not implemented and to fix slurmdb_jobs_get() database connection issue. There was no statement to close the database connection after fetching data, and as a result, the function was unable to fetch changes to the database running on flask server.

@ahmedhany93
Copy link
Author

It seems that the master branch already failed in the same steps.
Any way, I just wanted to ask if there was a reason for commenting out the edited options?
Also was there a reason for not closing the db_conn in slurdb_jobs_get ?

cwd = os.getcwd().encode("UTF-8", "replace")
desc.work_dir = cwd

if job_opts.get("work_dir"):
Copy link
Member

Choose a reason for hiding this comment

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

👍

dict J_dict = {}
slurm.List JOBSList
slurm.ListIterator iters = NULL
void* db_conn = slurm.slurmdb_connection_get()
Copy link
Member

Choose a reason for hiding this comment

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

👍 I can't remember if this got taken out or was missing to begin with.

# TODO: ntasks_set, cpus_set

if job_opts.get("ntasks"):
desc.min_cpus = job_opts.get("ntasks") * job_opts.get("cpus_per_task")
Copy link
Member

Choose a reason for hiding this comment

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

Is this right? I think there's some conditions to calculating min_cpus.

@giovtorres
Copy link
Member

The Python 2.6 tests need to get fixed. 😞

@giovtorres
Copy link
Member

Can you rebase please?

@giovtorres
Copy link
Member

No response. Please rebase. Closing for now.

@giovtorres giovtorres closed this Mar 13, 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

Successfully merging this pull request may close these issues.

2 participants