robertpofuk commented on code in PR #76467:
URL: https://github.com/apache/airflow/pull/76467#discussion_r4521375637
##########
airflow-core/src/airflow/api_fastapi/auth/tokens.py:
##########
@@ -113,13 +137,18 @@ class JWKS:
refresh_jwks: bool = True
refresh_interval_secs: int = 3600
refresh_retry_interval_secs: int = 10
+ default_algorithm: str | None = attrs.field(default=None,
validator=_validate_known_algorithm)
+ _load_locks: weakref.WeakKeyDictionary[asyncio.AbstractEventLoop,
asyncio.Lock] = attrs.field(
Review Comment:
Good catch, thanks. Fixed in 2548af43e1.
The per-loop `asyncio.Lock`s are gone. A load in flight is now one
`concurrent.futures.Future`, guarded by a `threading.Lock`, and shared by every
event loop and thread. A request on any loop that finds a load in flight awaits
it through `asyncio.wrap_future` before the retry-interval check, so it gets
the reloaded keyset instead of rejecting the new `kid`. The loop that starts
the load runs it and resolves the future when it finishes, whether it succeeds
or fails. The future is marked running, so a cancelled waiter can't cancel the
load for the others.
`test_request_on_another_event_loop_waits_for_reload_in_flight` covers your
scenario. It holds a reload for the rotated key in flight on a loop in another
thread, then requests the new `kid` on a second loop. That request waits for
the reload, gets the new key, and only one fetch is made. Against the previous
per-loop locks this test fails with `Key ID new not found in keyset`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
