dabla commented on code in PR #73414:
URL: https://github.com/apache/airflow/pull/73414#discussion_r4539818685
##########
airflow-core/src/airflow/dag_processing/manager.py:
##########
@@ -729,6 +731,10 @@ def _run_parsing_loop(self):
# needs to be done before this process is forked to create the DAG
parsing processes.
if self.api_client is None:
SecretCache.init()
+ # SDK lookups and the in-process API must share the cache
inherited by parsing forks.
+ ServerSecretCache._cache = SecretCache._cache
Review Comment:
**[nit]** The two caches are joined by copying private attributes from
outside the class.
Moving the cache to `shared/secrets_backend` gives core and the SDK each
their own `SecretCache` with its own `_cache` and `_ttl`. The manager then
makes them one cache again by assigning `ServerSecretCache._cache` and
`ServerSecretCache._ttl` by hand, and the resolver decides between a direct
call and a worker by reading `SecretCache._cache is None`. Both callers now
depend on how the shared class stores its state, and a third field added to it
later (a lock, a size limit) has to be copied here too or the two views drift
apart silently.
Since the shared class is new in this PR, it can own both operations:
```python
@classmethod
def share(cls, other: type[SecretCache]) -> None:
"""Use the cache ``other`` initialized, so both classes read and
invalidate the same entries."""
cls._cache = other._cache
if other._cache is not None:
cls._ttl = other._ttl
@classmethod
def is_initialized(cls) -> bool:
return cls._cache is not None
```
Then this block is `ServerSecretCache.share(SecretCache)` and
`_call_secret_cache` asks `SecretCache.is_initialized()`. The earlier reason
for reading `_cache` in the resolver ("no public initialization predicate, and
the SDK cache API is unchanged") no longer applies now that the class lives in
the shared library.
Related, same file set: `airflow.secrets.cache.SecretCache.init()` has no
caller outside the tests (the manager initializes the SDK class and shares it),
and its docstring says "configured for the API server", which never initializes
a cache. Either drop `init()` from the core adapter or say in the docstring
that the Dag processor is its only consumer.
--
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]
