pierrejeambrun commented on code in PR #73437:
URL: https://github.com/apache/airflow/pull/73437#discussion_r4083746357
##########
ts-sdk/tests/sdk/dag.test.ts:
##########
@@ -460,6 +459,84 @@ describe("Dag", () => {
expect(dag.taskIds).toEqual([]);
});
+ describe("an omitted task id", () => {
+ it("takes the handler's function name", () => {
+ const dag = new Dag("named_dag");
+ dag.task(async function extract() {})();
+
+ expect(dag.taskIds).toEqual(["extract"]);
+ });
+
+ it("takes the name of a handler declared elsewhere", () => {
+ async function transform() {}
+ const dag = new Dag("named_dag");
+ dag.task(transform)();
+
+ expect(dag.taskIds).toEqual(["transform"]);
+ });
+
+ it("still accepts a spec as the second argument", () => {
+ const dag = new Dag("named_dag");
+ dag.task(async function extract() {}, { retries: 2 })();
+
+ expect(getDagTaskRecords(dag).get("extract")?.spec).toEqual({ retries: 2
});
+ });
+
+ it("is taken from the spec when one names the task", () => {
+ const dag = new Dag("specced_id_dag");
+ dag.task(async function extract() {}, { taskId: "extract_rows" })();
+
+ expect(dag.taskIds).toEqual(["extract_rows"]);
+ });
+
+ it("prefers the positional id over the handler name", () => {
+ const dag = new Dag("positional_dag");
+ dag.task("extract_rows", async function extract() {})();
+
+ expect(dag.taskIds).toEqual(["extract_rows"]);
+ });
+
+ it("rejects a positional id and a spec id together", () => {
+ // The positional one used to win and the spec's was dropped in silence.
+ const dag = new Dag("two_ids_dag");
+
+ expect(() =>
+ dag.task("extract_rows", async function extract() {}, { taskId:
"from_spec" }),
+ ).toThrowError(
+ /Task "extract_rows" of Dag "two_ids_dag" also carries taskId
"from_spec" in its spec/,
+ );
+ expect(dag.taskIds).toEqual([]);
+ });
+
+ it("fails for an anonymous handler, naming the two ways to give it an id",
() => {
+ const dag = new Dag("anonymous_dag");
+
+ expect(() => dag.task(async () => 42)).toThrowError(
+ /A task of Dag "anonymous_dag" has no id: its handler is anonymous/,
+ );
+ expect(dag.taskIds).toEqual([]);
+ });
+
+ it("points at the packer when a handler's name was minified away", () => {
+ const dag = new Dag("minified_dag");
+ // What a bundle built without airflow-ts-pack's transform holds: the
+ // function is real, but esbuild took its name.
+ const minified = Object.defineProperty(async () => 42, "name", { value:
"" });
+
+ expect(() => dag.task(minified)).toThrowError(
+ /airflow-ts-pack resolves a named handler's id at pack time/,
+ );
+ });
Review Comment:
Ok, I get it, it's for ambiguous syntax.
##########
ts-sdk/tests/sdk/dag.test.ts:
##########
@@ -460,6 +459,84 @@ describe("Dag", () => {
expect(dag.taskIds).toEqual([]);
});
+ describe("an omitted task id", () => {
+ it("takes the handler's function name", () => {
+ const dag = new Dag("named_dag");
+ dag.task(async function extract() {})();
+
+ expect(dag.taskIds).toEqual(["extract"]);
+ });
+
+ it("takes the name of a handler declared elsewhere", () => {
+ async function transform() {}
+ const dag = new Dag("named_dag");
+ dag.task(transform)();
+
+ expect(dag.taskIds).toEqual(["transform"]);
+ });
+
+ it("still accepts a spec as the second argument", () => {
+ const dag = new Dag("named_dag");
+ dag.task(async function extract() {}, { retries: 2 })();
+
+ expect(getDagTaskRecords(dag).get("extract")?.spec).toEqual({ retries: 2
});
+ });
+
+ it("is taken from the spec when one names the task", () => {
+ const dag = new Dag("specced_id_dag");
+ dag.task(async function extract() {}, { taskId: "extract_rows" })();
+
+ expect(dag.taskIds).toEqual(["extract_rows"]);
+ });
+
+ it("prefers the positional id over the handler name", () => {
+ const dag = new Dag("positional_dag");
+ dag.task("extract_rows", async function extract() {})();
+
+ expect(dag.taskIds).toEqual(["extract_rows"]);
+ });
+
+ it("rejects a positional id and a spec id together", () => {
+ // The positional one used to win and the spec's was dropped in silence.
+ const dag = new Dag("two_ids_dag");
+
+ expect(() =>
+ dag.task("extract_rows", async function extract() {}, { taskId:
"from_spec" }),
+ ).toThrowError(
+ /Task "extract_rows" of Dag "two_ids_dag" also carries taskId
"from_spec" in its spec/,
+ );
+ expect(dag.taskIds).toEqual([]);
+ });
+
+ it("fails for an anonymous handler, naming the two ways to give it an id",
() => {
+ const dag = new Dag("anonymous_dag");
+
+ expect(() => dag.task(async () => 42)).toThrowError(
+ /A task of Dag "anonymous_dag" has no id: its handler is anonymous/,
+ );
+ expect(dag.taskIds).toEqual([]);
+ });
+
+ it("points at the packer when a handler's name was minified away", () => {
+ const dag = new Dag("minified_dag");
+ // What a bundle built without airflow-ts-pack's transform holds: the
+ // function is real, but esbuild took its name.
+ const minified = Object.defineProperty(async () => 42, "name", { value:
"" });
+
+ expect(() => dag.task(minified)).toThrowError(
+ /airflow-ts-pack resolves a named handler's id at pack time/,
+ );
+ });
Review Comment:
Ok, I get it, it's for ambiguous syntax. Check suggestion bellow.
--
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]
