Minor refactor to address self.client.submit_job(job) raising UnboundLocalError #57
Conversation
…b(job) raises an OSError, job_id is unassigned resulting in UnboundLocalError.
There was a problem hiding this comment.
Good catch, thanks for that! We can't do it this way, but I think this can be partially addressed with some small changes. However, the bigger questions to me are:
Why couldn't it submit the job?
What failed?
Should it be retried instead?
At the very least, I think it's helpful to know if this was a server unreachable situation (we should probably retry at least a bit for that) or something wrong with the job, in which case we should pass along what it said, if anything useful.
| @@ -146,14 +146,14 @@ def create_jobs(self): | |||
| try: | |||
| job_id = self.client.submit_job(job) | |||
| except OSError as exc: | |||
| logger.exception("Unable to create job: %s", job_id) | |||
| logger.exception("Unable to create job: %s", job) | |||
There was a problem hiding this comment.
You're right, we can't return the job_id here, but we shouldn't return the entire job json either. Perhaps the queue name that it would have gone on, and say "Unable to create job in queue: {...}"?
There was a problem hiding this comment.
That makes sense, I did not realize that we'd get the entire job json here.
Your other suggestion sounds reasonable to me - I'll try and implement this shortly.
There was a problem hiding this comment.
Were you still working on this patch?
|
@hum4n0id I don't know if this is still valid or not, but if it is, can you re-propose it against https://github.com/canonical/testflinger instead? |
Minor refactor in try, except block (146): when self.client.submit_job(job) raises an OSError, job_id is unassigned resulting in UnboundLocalError. This was brought to my attention this morning and I wanted to quickly address it.
Using job in place of job_id in the except block seemed appropriate as it's referencing a returned job_id two line above.