-
-
Notifications
You must be signed in to change notification settings - Fork 188
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.
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
Add workaround allowing resubmission #241
Conversation
As the commit/other information doesn't change when performing a rebuild, resubmissions will fail with an HTTP 422 error. This adds a workaround for such cases, appending a random value to the id and attempting a resubmission with that.
This should fix #238 |
@TheKevJames - feedback on this would be welcome. Not sure why ci/circleci is failing but otherwise looks ready for review to me. |
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.
This makes sense to me -- thanks for providing a fix for this (and apologies for taking so long to check out this PR!).
# failed due to resubmission (non-unique) | ||
if response.status_code == 422: | ||
self.config['service_job_id'] = '{}-{}'.format( | ||
self.config['service_job_id'], random.randint(0, sys.maxsize)) |
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.
Wouldn't this lead to a potential KeyError
on GitHub Actions?
Because the service_job_id
is only set if job
is set in Coveralls.__init__()
:
https://github.com/coveralls-clients/coveralls-python/blob/2.2.0/coveralls/api.py#L56-L57
However job
is always None
in Coveralls.load_config_from_github()
:
https://github.com/coveralls-clients/coveralls-python/blob/2.2.0/coveralls/api.py#L92-L109
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.
You may be right - looks like github actions are one of/the only ones that requires use of a token, and then submits this using the second method documented at https://docs.coveralls.io/api-introduction. Have you seen this occur in practice?
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 I think I saw it happening with when using service_name=github
and getting a 422.
The fix for the KeyError
was to simply provide service_job_id
a value (None
in my case) to the Coveralls
constructor. That was enough for me. See below for the fix including regression tests.
AndreMiras/coveralls-python-action@50af55e
Since it could well be me misusing the package, I guess we can simply acknowledge this bug/feature and take no actions until someone else complains
As the commit/other information doesn't change when performing a rebuild, resubmissions will fail with an HTTP 422 error. This adds a workaround for such cases, appending a random value to the id and attempting a resubmission with that.
As the commit/other information doesn't change when performing a
rebuild, resubmissions will fail with an HTTP 422 error. This adds a
workaround for such cases, appending a random value to the id and
attempting a resubmission with that.