shivaam commented on code in PR #72046:
URL: https://github.com/apache/airflow/pull/72046#discussion_r3868054523


##########
ts-sdk/src/cli/bundle-encoder.ts:
##########
@@ -0,0 +1,195 @@
+/*!
+ * 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.
+ */
+
+/**
+ * Encodes a self-contained TypeScript Dag bundle that remains directly
+ * executable by Node.
+ *
+ * Final byte order:
+ *
+ *   airflowBundle header
+ *   -> airflowMetadata
+ *   -> executable JavaScript
+ *
+ * The header tells Airflow where each region begins and ends and carries the
+ * digest used to verify each one. Metadata describes what the bundle can 
serve,
+ * and executable JavaScript runs its task handlers.
+ *
+ * This module owns the on-disk encoding. Readers must use the header's version
+ * and byte ranges rather than relying on incidental line positions.
+ */
+
+import { createHash } from "node:crypto";
+
+import type { BundleManifest } from "../coordinator/manifest.js";
+
+const AIRFLOW_BUNDLE_METADATA_VERSION = "1.0";
+const BUNDLE_LAYOUT_VERSION = 1;
+const EMBEDDED_METADATA_MAX_BYTES = 1024 * 1024;
+const OFFSET_HEX_WIDTH = 16;
+
+export const EMBEDDED_METADATA_PREFIX = "//# airflowMetadata=";
+export const EMBEDDED_LAYOUT_PREFIX = "//# airflowBundle=";
+
+export interface BundleEncoderInput {
+  bundleManifest: BundleManifest;
+  sdkVersion: string;
+  entrypointName: string;
+  executable: Uint8Array;
+}
+
+interface BundleMetadata {
+  airflow_bundle_metadata_version: "1.0";

Review Comment:
   Good catch. Updated this locally to `string` so the constant remains the 
single source of truth.
   
   ---
   Drafted-by: Codex; reviewed by @shivaam before posting



##########
ts-sdk/src/cli/bundle-encoder.ts:
##########
@@ -0,0 +1,195 @@
+/*!
+ * 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.
+ */
+
+/**
+ * Encodes a self-contained TypeScript Dag bundle that remains directly
+ * executable by Node.
+ *
+ * Final byte order:
+ *
+ *   airflowBundle header
+ *   -> airflowMetadata
+ *   -> executable JavaScript
+ *
+ * The header tells Airflow where each region begins and ends and carries the
+ * digest used to verify each one. Metadata describes what the bundle can 
serve,
+ * and executable JavaScript runs its task handlers.
+ *
+ * This module owns the on-disk encoding. Readers must use the header's version
+ * and byte ranges rather than relying on incidental line positions.
+ */
+
+import { createHash } from "node:crypto";
+
+import type { BundleManifest } from "../coordinator/manifest.js";
+
+const AIRFLOW_BUNDLE_METADATA_VERSION = "1.0";
+const BUNDLE_LAYOUT_VERSION = 1;
+const EMBEDDED_METADATA_MAX_BYTES = 1024 * 1024;
+const OFFSET_HEX_WIDTH = 16;
+
+export const EMBEDDED_METADATA_PREFIX = "//# airflowMetadata=";
+export const EMBEDDED_LAYOUT_PREFIX = "//# airflowBundle=";
+
+export interface BundleEncoderInput {
+  bundleManifest: BundleManifest;
+  sdkVersion: string;
+  entrypointName: string;
+  executable: Uint8Array;
+}
+
+interface BundleMetadata {
+  airflow_bundle_metadata_version: "1.0";
+  sdk: { language: string; version: string; supervisor_schema_version: string 
};
+  source: string;
+  dags: BundleManifest["dags"];
+}
+
+interface VerifiedByteRange {
+  end: string;
+  sha256: string;
+  start: string;
+}
+
+interface BundleHeader {
+  code: VerifiedByteRange;
+  metadata: VerifiedByteRange;
+  version: 1;

Review Comment:
   The two versions protect different contracts. The layout version describes 
the physical bundle structure—how the coordinator locates and verifies the 
metadata and executable sections. `airflow_bundle_metadata_version` describes 
the logical schema inside the metadata section.
   
   Keeping them separate also allows the layout header to carry an independent 
digest for the metadata section. This lets the coordinator detect metadata 
corruption as well as executable corruption. I think the separation will also 
help if we later add source, signatures, or other physical sections.
   
   ---
   Drafted-by: Codex; reviewed by @shivaam before posting



##########
ts-sdk/src/cli/bundle-encoder.ts:
##########
@@ -0,0 +1,195 @@
+/*!
+ * 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.
+ */
+
+/**
+ * Encodes a self-contained TypeScript Dag bundle that remains directly
+ * executable by Node.
+ *
+ * Final byte order:
+ *
+ *   airflowBundle header
+ *   -> airflowMetadata
+ *   -> executable JavaScript
+ *
+ * The header tells Airflow where each region begins and ends and carries the
+ * digest used to verify each one. Metadata describes what the bundle can 
serve,
+ * and executable JavaScript runs its task handlers.
+ *
+ * This module owns the on-disk encoding. Readers must use the header's version
+ * and byte ranges rather than relying on incidental line positions.
+ */
+
+import { createHash } from "node:crypto";
+
+import type { BundleManifest } from "../coordinator/manifest.js";
+
+const AIRFLOW_BUNDLE_METADATA_VERSION = "1.0";
+const BUNDLE_LAYOUT_VERSION = 1;
+const EMBEDDED_METADATA_MAX_BYTES = 1024 * 1024;
+const OFFSET_HEX_WIDTH = 16;
+
+export const EMBEDDED_METADATA_PREFIX = "//# airflowMetadata=";
+export const EMBEDDED_LAYOUT_PREFIX = "//# airflowBundle=";
+
+export interface BundleEncoderInput {
+  bundleManifest: BundleManifest;
+  sdkVersion: string;
+  entrypointName: string;
+  executable: Uint8Array;
+}
+
+interface BundleMetadata {
+  airflow_bundle_metadata_version: "1.0";
+  sdk: { language: string; version: string; supervisor_schema_version: string 
};
+  source: string;
+  dags: BundleManifest["dags"];
+}
+
+interface VerifiedByteRange {
+  end: string;
+  sha256: string;
+  start: string;
+}
+
+interface BundleHeader {
+  code: VerifiedByteRange;
+  metadata: VerifiedByteRange;
+  version: 1;
+}
+
+export function encodeBundle(input: BundleEncoderInput): Buffer {
+  const metadata = encodeMetadata(input);
+  const executable = encodeExecutable(input.executable);
+  const header = encodeHeader({ metadata, executable });
+
+  return Buffer.concat([header, metadata, executable]);
+}
+
+function encodeHeader(regions: { metadata: Buffer; executable: Buffer }): 
Buffer {
+  const metadataPayload = regions.metadata.subarray(
+    Buffer.byteLength(EMBEDDED_METADATA_PREFIX),
+    -1,
+  );
+  const digests = {
+    code: computeSha256(regions.executable),
+    metadata: computeSha256(metadataPayload),
+  };
+  const zeroOffset = "0".repeat(OFFSET_HEX_WIDTH);
+  const placeholderHeader = renderHeader({
+    version: BUNDLE_LAYOUT_VERSION,
+    code: { start: zeroOffset, end: zeroOffset, sha256: digests.code },
+    metadata: { start: zeroOffset, end: zeroOffset, sha256: digests.metadata },
+  });
+  const metadataStart = placeholderHeader.length + 
Buffer.byteLength(EMBEDDED_METADATA_PREFIX);
+  const metadataEnd = metadataStart + metadataPayload.length;
+  const codeStart = placeholderHeader.length + regions.metadata.length;
+  const codeEnd = codeStart + regions.executable.length;
+  const header = renderHeader({
+    version: BUNDLE_LAYOUT_VERSION,
+    code: {
+      start: formatOffset(codeStart),
+      end: formatOffset(codeEnd),
+      sha256: digests.code,
+    },
+    metadata: {
+      start: formatOffset(metadataStart),
+      end: formatOffset(metadataEnd),
+      sha256: digests.metadata,
+    },
+  });
+  if (header.length !== placeholderHeader.length) {
+    throw new Error("Bundle header changed length while resolving section 
offsets");
+  }
+  return header;
+}
+
+function encodeMetadata(input: BundleEncoderInput): Buffer {
+  const payload = Buffer.from(
+    Buffer.from(renderMetadata(buildBundleMetadata(input)), 
"utf-8").toString("base64"),

Review Comment:
   Good point. I kept the base64 encoding to support the YAML-based metadata 
format added by the original TypeScript packer. Since the runtime already 
produces structured JSON and the TypeScript SDK is unreleased, I think it makes 
sense to simplify this to compact JSON embedded directly in the single-line 
comment and remove the metadata base64 encoding.
   
   ---
   Drafted-by: Codex; reviewed by @shivaam before posting



-- 
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]

Reply via email to