Add airflow-ts-pack bundle build tool for TypeScript SDK#69295
Conversation
jason810496
left a comment
jason810496
left a comment
There was a problem hiding this comment.
Thanks! Would like to know more about the design choice before going down the implementation details.
4583f0e to
9031129
Compare
jason810496
left a comment
jason810496
left a comment
There was a problem hiding this comment.
Nice! Thanks for the follow-up and sorry that I didn't think about the trailer design decision in the first place.
https://github.com/apache/airflow/pull/69295/changes#r3551043871 is the only comment from me. Rest of them are nits raise by CC, please feel free to push back if the comment doesn't make sense to you. Thanks!
|
@jason810496 Could we hold off on merging this until I finish reviewing it? This PR defines an important user-facing build and packaging workflow, and I want to ensure it fits well with existing TypeScript projects. |
Sure, we still have some buffer to wait for review. I target to merge this one at the beginning of next week (Mon or Tue). |
|
Not a blocker and can be done later if you want. Projects or teams may use Vite, webpack, or custom esbuild plugins and settings. Could we add a packaging-only mode for an existing .mjs artifact? In this mode, ts-pack would not compile the code again. It would only run the bundle’s metadata introspection and embed the Airflow metadata. We can keep the simple command as the default option and the packaging only mode can be the second option. The only requirement on the user's build output is that the .mjs is a single file node can run directly, with all dependencies bundled inside. |
Address review: strip entry shebangs so the metadata line stays first, read the manifest via a sentinel line immune to import-time logging, validate before writing bundle.mjs, make esbuild an optional peer dependency, and reject null dags manifests.
|
I like this idea. One small note: introspection needs the artifact to reach |
sounds good |
shivaam
left a comment
shivaam
left a comment
There was a problem hiding this comment.
Thanks for addressing the comments. Looks good.
* Add airflow-ts-pack bundle build tool for TypeScript SDK * Embed TypeScript bundle metadata as leading comment * Harden airflow-ts-pack manifest reading and packaging Address review: strip entry shebangs so the metadata line stays first, read the manifest via a sentinel line immune to import-time logging, validate before writing bundle.mjs, make esbuild an optional peer dependency, and reject null dags manifests. * Validate embedded metadata size and split manifest out of runtime
3 participants
Related Issue
close #69286
Why
TypeScript SDK users had to run esbuild by hand and hand-write the airflow-metadata.yaml sidecar, which could drift from the bundle.
How
airflow-ts-packCLI that bundles a TypeScript entrypoint withesbuildand embeds the Airflow metadata intobundle.mjsas a trailing//# airflowMetadata=<base64>comment, producing a single deployable artifact--airflow-metadatadump mode tostartCoordinator()so the manifest comes from the bundle's own task registry, never hand-writtenNodeCoordinatorto read embedded metadata first, falling back to theairflow-metadata.yamlsidecar for compatibilityWas generative AI tooling used to co-author this PR?
Claude Code with Fable 5
{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.