PrakshiGoyal10 commented on PR #69998: URL: https://github.com/apache/airflow/pull/69998#issuecomment-5344218176
Thanks both — this is a great catch, and the `SerializedDagModel.get_dag` round-trip is exactly the kind of test I should have had; my link tests ran against a `Mock` operator and never exercised the deserialized path, so both blockers were invisible to them. @Vamsi-klu — yes please, go ahead and take a pass at these on this PR; I'd rather build on your fix than have you open a competing one. A patch I can pull works well. On approach: I don't have a strong preference between persisting the URLs into XCom (matches the job-run link precedent) and @moomindani's suggestion of declaring `operators` on the two link classes so the real link objects survive deserialization and `_build_repair_url` runs at request time. The latter is appealing because it avoids freezing `base_url` at execute time and actually exercises the branches already in the PR — but I'll defer to whichever you and @moomindani think is cleaner. Appreciate the thorough reviews. --- Drafted-by: Claude Code (Opus 4.8); reviewed by @PrakshiGoyal10 -- 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]
