fix: Updated Error Type to TaskclusterRestFailure to align with the switch to the taskcluster package #822
Conversation
bhearsum
left a comment
bhearsum
left a comment
There was a problem hiding this comment.
Are there other places we're catching errors from the old functions? If you haven't done an audit of them, please do before merging this.
bhearsum
left a comment
bhearsum
left a comment
There was a problem hiding this comment.
It looks like we might need to catch both of these?
abhishekmadan30
force-pushed
the
error-fix
branch
from
76d2062 to
141c420
Compare
I will look through it and see if there is anything, I can change rn |
ahal
left a comment
ahal
left a comment
There was a problem hiding this comment.
Assuming you've done a quick scan for other such cases, this lgtm!
abhishekmadan30
force-pushed
the
error-fix
branch
from
141c420 to
2080104
Compare
| cancel_task(task_id) | ||
| except requests.HTTPError as e: | ||
| if e.response.status_code == 409: | ||
| except (requests.HTTPError, TaskclusterRestFailure) as e: |
There was a problem hiding this comment.
Do we still need to catch HTTPError? I didn't think it would still be possible to get this, same Q for the other two places.
There was a problem hiding this comment.
I don't think is possible either, I wanted to keep to air on the side of caution and remove it a later in a less sensitive time
There was a problem hiding this comment.
Ok sounds good, I don't see any harm in leaving it for now.. Though maybe file an issue to clean it up later?
abhishekmadan30
force-pushed
the
error-fix
branch
from
2080104 to
ff7209b
Compare
| cancel_task(task_id) | ||
| except requests.HTTPError as e: | ||
| if e.response.status_code == 409: | ||
| except (requests.HTTPError, TaskclusterRestFailure) as e: |
There was a problem hiding this comment.
Ok sounds good, I don't see any harm in leaving it for now.. Though maybe file an issue to clean it up later?
…witch to the taskcluster package
3 participants
…witch to the taskcluster package