samraj2k commented on PR #73209: URL: https://github.com/apache/airflow/pull/73209#issuecomment-6223513209
> Thanks for the work on this stack. I'd like to suggest a different split: > > 1. Make #73217 the reload-only PR, based on main: the Dag processor re-reads > `dag_bundle_config_list` on an opt-in interval, the key is marked `sensitive: true` > so `_cmd` / `_secret` can supply a changing value, and your reconcile loop does the rest. > A failed or empty read must keep the last good list, and `--bundle-name` processors > keep today's never-deactivate behaviour. No new option, no new class, no change to > `DagFileParseRequest`. > 2. Keep this PR for `DagBundleSource` (with its docs, the `team_name` protocol change and > the `provides_complete_bundle_list` flag), gated on an AIP or an AIP-66 amendment. > > Reasons: > > * `DagBundleSource` is not needed for hot reload. The reconcile loop in #73217 only calls > `DagBundlesManager` methods, and the default source in this stack keeps a cached snapshot, > so as written nothing reloads unless a custom class is installed. > * A new public extension point loaded in every component is what the AIP process exists for > (compare AIP-85 for Dag importers). Jarek asked for one in #61645. Reload of an existing > option does not need that bar. > > For the reload-only PR, I think a dev-list [DISCUSS] thread that spells out deactivation, failure handling and multi-processor behaviour, followed by lazy consensus, is sufficient. I am happy to start or co-sign that thread. > > @judahrand, since #71111 targets the same problem, would you be open to joining the reload PR? > > -- > > Drafted with Claude Fable 5, reviewed by me. Thanks a lot @shahar1 for the comment. I would explain my I kept the bundle source PR first. We already have many use case of reading dynamic bundle: through file, API etc, and having this configurable source allows a native support. Though, I completely agree that _CMD can support the use cases. For now, as per comment I will make changes to have hot reload capability directly on main. @judahrand I will be working on this, and would implement using _CMD (making sensitive), let me know if you have any concerns -- 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]
