Repository navigation
Add the manifest format for manifest-backed versioned Dag bundles - #72476
Open
ephraimbuddy wants to merge 5 commits into
Open
ephraimbuddy wants to merge 5 commits into
ephraimbuddy wants to merge 5 commits into
Conversation
ephraimbuddy
changed the title
ephraimbuddy
force-pushed
the
add-manifest-bundle-primitives
branch
from
September 10, 2026 19:02
310fb7b to
c93a7f7
Compare
phanikumv
reviewed
Sep 22, 2026
phanikumv
reviewed
Sep 22, 2026
ephraimbuddy
force-pushed
the
add-manifest-bundle-primitives
branch
from
October 7, 2026 10:19
4ab9e24 to
bc0b815
Compare
ferruzzi
approved these changes
Oct 7, 2026
ferruzzi
left a comment
ferruzzi
left a comment
Contributor
There was a problem hiding this comment.
Couple non-blocking thoughts, otherwise thanks for the version_id addition.
I like the error kind split that you added as well, that's a clean solution
| not check the manifest against its own contents; a caller that did not choose | ||
| ``expected_sha256`` itself must additionally confirm that ``manifest["version"]`` | ||
| equals ``compute_bundle_version(manifest["files"])``. | ||
| """ |
Contributor
There was a problem hiding this comment.
Non-blocking: Is there a reason to only verify the sha and nott he contents? Is that a security thing? IIRC your PoC had a validator that compared more thoroughly?
One reason I ask if because more thorough checking here cold confirm the structure and make the KeyError less likely (maybe impossible, but users will always find some new way to break things) as an alternative to my comment above about changing the error type.
Contributor
Author
There was a problem hiding this comment.
You are right. The digest check alone wasn't enough, so I've brought the PoC's validator back with stricter checks
Contributor
Author
There was a problem hiding this comment.
You might want to take a look again
This is the first of several changes that add manifest-backed Dag bundles. A manifest lists every file in a bundle with its checksum, and the bundle version is a hash of that list. Bundles get published and read on different machines, so both sides have to agree on how the list is written and on what makes a version or a file path valid. Keeping those rules in one place lets the storage backends that come next share them instead of each keeping its own copy.
Reading a Dag bundle's source tree can fail for two different reasons. A file that vanished mid-walk means someone edited the Dag folder while we were looking at it, and a publisher should simply try again on its next pass. A permission problem or a stale network handle is a standing fault that an operator has to see and fix. Only the first was being caught, so the second escaped as a raw OSError instead of a bundle error, and folding both into the "source changed" error would leave a misconfigured Dag folder pinned to a stale version with nothing but a debug log to show for it.
An object store can keep several versions of the same key, so the key a release was built from is not necessarily the one a consumer fetches later. Carrying the backend's own identifier for each stored object, such as an S3 VersionId, leaves room to pin a fetch to the exact object instead of trusting whatever currently sits at the key. Nothing populates it yet; it is optional so the local backend, which has no such identifier, is unaffected. It is deliberately excluded from the bundle version: republishing identical files to a versioned bucket mints new object versions, and that must still resolve to one bundle version.
Every other read of a bundle source now reports trouble as a manifest error, but a missing or unreadable root still escaped as a bare OSError, so a caller that guards a publish with the module's own error type would not catch it. Unlike a file vanishing mid-walk, a root we never saw is a configured path rather than a concurrent edit, so none of these say the source changed: a typo in the configuration must not look like something worth quietly retrying. Backend object versions are also checked before the hashing pass rather than during it, so a bad identifier no longer costs a full read of the tree first.
A digest only establishes that a manifest is the document it was taken from, and a consumer reading a manifest had no way to check the file entries against anything. Reading a malformed entry also failed as a stray KeyError rather than a manifest error. Re-deriving the version from the entries binds the two together, which is an integrity guarantee only against a version the publisher did not supply, such as an operator's pin or one Airflow already recorded; checked against nothing but itself, a manifest is merely self-consistent. Making the caller state which case applies keeps the weaker one from being the default, and fixing the entry order keeps one set of files from being publishable under several versions.
ephraimbuddy
force-pushed
the
add-manifest-bundle-primitives
branch
from
October 8, 2026 13:54
fa1d563 to
a30fcde
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the first of several changes that add manifest-backed Dag bundles. A manifest lists every file in a bundle with its checksum, and the bundle version is a hash of that list.
Bundles get published and read on different machines, so both sides have to agree on how the list is written and on what makes a version or a file path valid. Keeping those rules in one place lets the storage backends that come next share them instead of each keeping its own copy.
POC: https://github.com/ephraimbuddy/airflow-manifest-bundle
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Opus 5 following the guidelines
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.