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.

Sign up for GitHub

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

quick fixes #153

Merged
merged 2 commits into from
Oct 1, 2024
Merged

quick fixes #153

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3246c4d
quick fixes
charles-cowart Sep 19, 2024
154d811
Removed unneeded change
charles-cowart Oct 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions sequence_processing_pipeline/Job.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,9 @@ def wait_on_job_ids(self, job_ids, callback=None):
# them before returning, optionally submitting callbacks for each
# job-id.

# ensure all ids are strings to ensure proper working w/join().
job_ids = [str(x) for x in job_ids]
Copy link
Contributor

Choose a reason for hiding this comment

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

What edge case caused problems here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I once passed in a list of job-ids that had been converted to int()s in the process of error checking and it made the entire job fail because of a downstream str.join(). IMHO this is prudent as there's nothing else checking the input parameters for this method and this kind of error is the kind that can be hard to debug for some down the road. Moreover because it uses a list comprehension it should be pretty cheap.


def query_slurm(job_ids):
# internal function query_slurm encapsulates the handling of
# squeue.
Expand Down Expand Up @@ -257,6 +260,8 @@ def query_slurm(job_ids):
jobs = {}
child_jobs = {}
for job_id, unique_id, state in lines:
# ensure unique_id is of type string for downstream use.
unique_id = str(unique_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this actually possible? I mean, lines is an array of strings (x.split).

jobs[unique_id] = state

if unique_id != job_id:
Expand Down
Loading