guan404ming commented on code in PR #69295: URL: https://github.com/apache/airflow/pull/69295#discussion_r3570968145
########## task-sdk/src/airflow/sdk/coordinators/node/coordinator.py: ########## @@ -45,6 +46,34 @@ BUNDLE_FILENAME = "bundle.mjs" METADATA_FILENAME = "airflow-metadata.yaml" +EMBEDDED_METADATA_MARKER = b"//# airflowMetadata=" +# Metadata sits on the bundle's first line; a bounded read is enough. +EMBEDDED_METADATA_HEAD_BYTES = 1 << 20 Review Comment: Fixed, changed to `1024 * 1024` and renamed to `EMBEDDED_METADATA_MAX_BYTES`. ########## ts-sdk/src/cli/pack.ts: ########## @@ -0,0 +1,218 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// airflow-ts-pack: bundle a TypeScript entrypoint into the single-file +// artifact NodeCoordinator consumes — `bundle.mjs` with the airflow +// metadata embedded as a leading `//# airflowMetadata=<base64>` comment. +// +// Mirrors airflow-go-pack: build first, then run the built bundle with +// --airflow-metadata so the manifest comes from the bundle's own task +// registry and schema version, never from a hand-written sidecar. + +import { execFileSync } from "node:child_process"; +import { readFileSync, rmSync, writeFileSync } from "node:fs"; +import path from "node:path"; + +import { + AIRFLOW_METADATA_FLAG, + AIRFLOW_METADATA_SENTINEL, + type BundleManifest, +} from "../coordinator/runtime.js"; + +const AIRFLOW_BUNDLE_METADATA_VERSION = "1.0"; +const BUNDLE_FILENAME = "bundle.mjs"; +// bundle.mjs is written only after validation, so failures leave no partial artifact. +const STAGING_FILENAME = "bundle.pack-staging.mjs"; +const MANIFEST_TIMEOUT_MS = 60_000; +const MANIFEST_MAX_BUFFER = 64 << 20; Review Comment: Fixed, renamed to `MANIFEST_MAX_BUFFER_BYTES = 64 * 1024 * 1024`. ########## task-sdk/src/airflow/sdk/coordinators/node/coordinator.py: ########## @@ -45,6 +46,34 @@ BUNDLE_FILENAME = "bundle.mjs" METADATA_FILENAME = "airflow-metadata.yaml" +EMBEDDED_METADATA_MARKER = b"//# airflowMetadata=" +# Metadata sits on the bundle's first line; a bounded read is enough. +EMBEDDED_METADATA_HEAD_BYTES = 1 << 20 + + +def _read_embedded_metadata(bundle_path: pathlib.Path) -> dict[str, Any] | None: + """ + Read the manifest ``airflow-ts-pack`` embeds in the bundle itself. + + The packer prepends the ``airflow-metadata.yaml`` content as a leading + ``//# airflowMetadata=<base64>`` line comment, keeping bundle and metadata + a single artifact. Returns ``None`` when the bundle has no such marker. + """ + try: + with bundle_path.open("rb") as bundle_file: + head = bundle_file.read(EMBEDDED_METADATA_HEAD_BYTES) + except OSError as exc: + raise ValueError(f"cannot read {bundle_path.name}: {exc}") from exc + + if not head.startswith(EMBEDDED_METADATA_MARKER): + return None + + payload = head[len(EMBEDDED_METADATA_MARKER) :].split(b"\n", 1)[0].strip() + try: + decoded = base64.b64decode(payload, validate=True) + except ValueError as exc: + raise ValueError(f"cannot parse embedded airflow metadata: {exc}") from exc Review Comment: Fixed. The packer now rejects metadata over 1 MiB before writing `bundle.mjs`, so this surfaces at build time. ########## task-sdk/src/airflow/sdk/coordinators/node/coordinator.py: ########## @@ -45,6 +46,34 @@ BUNDLE_FILENAME = "bundle.mjs" METADATA_FILENAME = "airflow-metadata.yaml" +EMBEDDED_METADATA_MARKER = b"//# airflowMetadata=" +# Metadata sits on the bundle's first line; a bounded read is enough. +EMBEDDED_METADATA_HEAD_BYTES = 1 << 20 + + +def _read_embedded_metadata(bundle_path: pathlib.Path) -> dict[str, Any] | None: + """ + Read the manifest ``airflow-ts-pack`` embeds in the bundle itself. + + The packer prepends the ``airflow-metadata.yaml`` content as a leading + ``//# airflowMetadata=<base64>`` line comment, keeping bundle and metadata + a single artifact. Returns ``None`` when the bundle has no such marker. + """ + try: + with bundle_path.open("rb") as bundle_file: + head = bundle_file.read(EMBEDDED_METADATA_HEAD_BYTES) Review Comment: Fixed, switched to a bounded `readline()` that raises an explicit error when the metadata line exceeds 1 MiB. ########## ts-sdk/src/coordinator/runtime.ts: ########## @@ -103,10 +103,37 @@ export function parseArgs(argv: readonly string[]): ParsedArgs { return { commAddr, logsAddr }; } +export const AIRFLOW_METADATA_FLAG = "--airflow-metadata"; + +/** Marks the manifest line on stdout, which import-time logging may also reach. */ +export const AIRFLOW_METADATA_SENTINEL = "__AIRFLOW_METADATA__ "; + +/** Bundle manifest fields only the built bundle itself knows: the schema + * version it was compiled against and the Dag/task pairs it registered. + * `airflow-ts-pack` runs `node bundle.mjs --airflow-metadata` to read this. */ +export interface BundleManifest { + supervisor_schema_version: string; + dags: Record<string, { tasks: string[] }>; +} Review Comment: Done, moved `BundleManifest`, `buildBundleManifest`, the flag and sentinel to `manifest.ts`. -- 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]
