jira
jira copied to clipboard
refactor 401 retrying into both auth methods
See #1904 for details on why.
This change refactors 401 retrying into the common base class, and for token auth the retry is implemented by clearing the session cookie and retrying with the same PAT as recommended.
In a few cases, the code is kind of ugly because in order to implement the retry I need to have a reference to the session object, which is only available if the tokenauth is created through the normal client constructor. If the tokenauth is created directly then the session might not be available. I am not sure if this is a supported case, if it's not then I might be able to clean up the checks for session and the asserts.
@ssbarnea not sure what to do about the check failures, can you advise?
Hi @adehad!
Can you please review this PR?
Thank you!
Hi,
This is now impacting multiple workflows on our side. Can you please merge this? Seems like a simple change (compared of my PRs :D ).
@adehad I agree with both of your requested changes but just wanted to give you a heads up that we have our internal Jira 10 upgrade planned for next month, and if you look at the linked issue, it appears that someone found an upstream issue that matches the symptoms I was seeing here, that's fixed in Jira 10.
So, I may not get back to this PR, and honestly, you would be forgiven for closing this PR outright, since it's just a workaround for a bug that's resolved in the latest supported major version of the non-cloud Jira software (I don't know what your support policy is for older versions of the Jira server).
Thanks @gmishkin for this update, I didn't catch that.
Sorry for the delay in getting this merged when it would have made the most impact to you.
I think there is still value in keeping it open and even merging, as I can imagine there are individuals affected that won't get the change to migrate as soon. Happy to leave that decision to you or any other budding contributors and happy to facilitate. (Based on this it may also be helpful to bring attention to that information in the code to help someone understand why we might have gone down this route, maybe pointing to the upstream issue / this PR)
I've just come back to open source contributions after a long pause, but now that I have time available I should be able to accelerate merging if it is ready.
@adehad I don't know if this is a thing that exists on GitHub, but if you're able to you should "pin" this comment
Adding a comment to the code describing what it's working around makes sense too 👍
P.S. no worries about not getting it merged, we have an internal pypi instance I've been publishing this so that our internal apps can use.