AP-807: Unpins authlib w/client_id header allowed - #94
Conversation
|
@anarchivist Review requested, but I think we should get this on staging temporarily before merging it. |
anarchivist
left a comment
anarchivist
left a comment
There was a problem hiding this comment.
r+; one minor clarification requested.
| "apache-airflow-task-sdk == 1.3.0", | ||
| # Temporary workaround for https://github.com/authlib/authlib/issues/902. | ||
| "authlib < 1.7.0", | ||
| "authlib", |
There was a problem hiding this comment.
does this effectively pin us to authlib >= 1.7.0? i think that's when it changed to call joserfc directly.
There was a problem hiding this comment.
I don't know if it does. The test may fail on Authlib < 1.7.0, but the code itself should work fine. (the test might still work actually? can't properly tell.)
If we are < 1.7.0, we are using the old code and it doesn't matter whether the header is registered or not. We are pulling in joserfc ourselves so the dependency isn't transient - the registration will succeed, it is just not used anywhere. If we are >= 1.7.0, then the registration fixes the client_id issue and we're good.
bcd521e to
5cedced
Compare
|
MR for |
|
Confirming it worked on staging, so merging. |
3 participants
No description provided.