AP24110011695 commented on PR #76481: URL: https://github.com/apache/airflow/pull/76481#issuecomment-6862559691
Thank you for this review — it's exactly the verification this PR needed, and I've confirmed both points on current main: - `task-sdk/src/airflow/sdk/__init__.pyi` re-exports `SecretCache` (line 140, `SecretCache as SecretCache`, and in the stub's `__all__` at line 211), so mypy reading the stub already accepts the import — the "type checkers flag it" motivation in the PR description does not hold for main today. - `tests/task_sdk/docs/test_public_api.py` deliberately exempts `SecretCache` by name in two places (the `dir(sdk)` ↔ `__all__` comparison at line 56, and `test_lazy_imports_match_public_api`'s `ignore` set at line 95). Two named exemptions look like a deliberate decision to keep it off the star-import surface, not drift. So I concede the framing: this PR's real effect is promoting `SecretCache` to public API, which is a maintainer decision rather than a bug fix — and the existing exemptions are evidence the current state is intended. You're also right that `test_lazy_imports_match_public_api` already covers what our added test checks once the exemption is removed. Unless the maintainers actually want `SecretCache` as public API (in which case the two exemptions can be dropped and `task-sdk/docs/api.rst` gains an entry, as you outlined), we'll close this. Thanks again for doing the run-on-main work — it settled the question cleanly. -- 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]
