kjh0623 commented on PR #70003: URL: https://github.com/apache/airflow/pull/70003#issuecomment-5018479154
@jedcunningham On losing the versioning benefits — you're right that `GitDagBundle` brings DagVersioning (each commit becomes a `DagVersion`, and task runs are pinned to the version they were queued with), and that's a real benefit. For deployments that want commit-pinned execution it's the right choice, and nothing here argues against it. The thing is, git-sync + `LocalDagBundle` is a separate, already-shipped pattern — it's what the Helm chart deploys by default when `dags.gitSync` is enabled (the default bundle is a `LocalDagBundle` over the dags folder, with git-sync swapping the checkout underneath it). Deployments on it have already traded per-commit versioning for centralized sync and no per-pod clone cold-start on large repos. This PR doesn't push anyone toward that trade or away from `GitDagBundle`; it just fixes a correctness bug — mid-round cross-commit reads — that today silently affects everyone already on that pattern, including default-Helm gitSync users. So "use `GitDagBundle` to keep versioning" is fair advice, but it doesn't help the deployments that are on git-sync right now and currently hit silent cross-commit tearing — telling them not to use git-sync isn't a fix for the bug they're hitting. The two are orthogonal: this opt-in flag (default `False`) makes the git-sync path *correct*; it doesn't make it *preferable*. -- 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]
