-
Notifications
You must be signed in to change notification settings - Fork 4
Conversation
…e test for the new functionality
…o wait for before processing the resutls
running | ||
""" | ||
# Get all the commands running pf the current user | ||
user = os.environ['USER'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is against PEP 8 recommendations. You should use environ[...]
vs. os.environ[...]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I'm assuming that I've to do the same for the Popen
call below, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, for everything in your code.
@antgonza I think I've addressed your comments. Let me know if there is anything left. |
lines = stdout.splitlines() | ||
running_jobs = [] | ||
for l in lines: | ||
job_id, job_name, user, time, status, queue = l.split() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will it be possible to ignore empty lines and those that start with # (comments).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should never happen. Note that lines is the output of qstat | grep user
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong line, meant to add it to were the parsing of the text file to create jobs happens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The creation of jobs happens in bash, so I don't really know where you meant 😖
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, my bad. Was confused with the way that cluster_jobs.py works.
I have all my comments in. @wasade do you have any? |
@wasade Are you going to review this? |
After 12 days @wasade didn't reply. Thus, I assume that he has no more comments. I will merge now. |
Fixes #20
Fixes #18
Allows the creation of a benchmark suite for a queueing system.
Adds a
wait_on
functionality on theprocess-bench-results
command, which receives a list of job ids and waits for its completion before processing the results.@antgonza, @wasade are you available for review?