bbovenzi commented on issue #76370:
URL: https://github.com/apache/airflow/issues/76370#issuecomment-6666403320

   A few thoughts:
   
   I think there are too many steps to declare a task:
   ```
   const summarize = dag.task("summarize", async (...) => ...); // factory
   const summary   = summarize({ eu: eu(), us: us() });         // TaskRef
   ```
   We write summar(y/ize) three times. Then we also call `eu` so many times 
too. Also, `const summary = summarize({ eu: eu(), us: us() })` reads like we 
are mixing execution inside of a declaration. 
   
   
   
   We're mixing classes and factory functions. I'd prefer we only do factory 
functions. I can't tell if `Dag` is a type or a class.
   
   
   Our task name declaration is inconsistent which makes it hard to read the 
code.
   `dag.if(hasOrders, { s: summary }, { taskId: "has_orders" })` should be 
`dag.if("has_orders", ...`
   Also `.then(publish())` doesnt not read like proper async chaining.
   
   Here is what I think the file shoudl look like:
   
   
   ```
   import { defineBundle, defineDag, getClient } from "@apache-airflow/ts-sdk";
   
   type Orders = { region: string; count: number };
   type Summary = { total: number };
   
   // Task bodies: plain functions
   const fetchEu = (): Orders => ({ region: "eu", count: 120 });
   const fetchUs = (): Orders => ({ region: "us", count: 80 });
   const summarize = ({ eu, us }: { eu: Orders; us: Orders }): Summary => ({
     total: eu.count + us.count,
   });
   const publish = async () => {
     const channel = await getClient().getVariable("report_channel");
     console.log(`Publishing the daily report to ${channel ?? "#data"}`);
   };
   const nothingToReport = () => console.log("No orders today");
   const hasOrders = ({ summary }: { summary: Summary }) => summary.total > 0;
   
   // The Dag: places tasks
   const ordersReport = defineDag("ts_orders_report", { schedule: "@daily", 
catchup: false }, (dag) => {
     const extract = dag.group("extract", (group) => ({
       eu: group.task("eu", fetchEu),
       us: group.task("us", fetchUs),
     }));
     const summary = dag.task("summarize", summarize, extract);
   
     dag.if("has_orders", {
       inputs: { summary },
       condition: hasOrders,
       then: dag.task("publish", publish),
       else: dag.task("nothing_to_report", nothingToReport),
     });
   });
   
   const bundle = defineBundle(ordersReport);
   await bundle.serve();
   ```
   


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