jason810496 commented on code in PR #55257:
URL: https://github.com/apache/airflow/pull/55257#discussion_r2349277076
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_extra_links.py:
##########
@@ -305,3 +307,72 @@ def test_should_respond_404_invalid_map_index(self,
test_client):
)
assert response.status_code == 404
assert response.json() == {"detail": "TaskInstance not found"}
+
+ def test_parameterized_extra_links_dag(self, dag_maker, test_client,
session):
+ with dag_maker(dag_id="parameterized_extra_links_dag",
serialized=True):
+
+ @task
+ def get_api_resource(extra_links: dict[str, str]):
+ rsp = requests.get("https://www.google.com";, timeout=10)
+ extra_links["Google"] = "https://www.google.com";
+ return {"status_code": rsp.status_code}
Review Comment:
Should we mock the `requests` or just set the `extra_links` directly to
avoid flaky test?
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_extra_links.py:
##########
@@ -305,3 +307,72 @@ def test_should_respond_404_invalid_map_index(self,
test_client):
)
assert response.status_code == 404
assert response.json() == {"detail": "TaskInstance not found"}
+
+ def test_parameterized_extra_links_dag(self, dag_maker, test_client,
session):
+ with dag_maker(dag_id="parameterized_extra_links_dag",
serialized=True):
+
+ @task
+ def get_api_resource(extra_links: dict[str, str]):
+ rsp = requests.get("https://www.google.com";, timeout=10)
+ extra_links["Google"] = "https://www.google.com";
+ return {"status_code": rsp.status_code}
+
+ get_api_resource(extra_links={})
+
+ dagrun = dag_maker.create_dagrun(
+ run_id="test_run",
+ logical_date=self.default_time,
+ state=DagRunState.RUNNING,
+ run_type=DagRunType.MANUAL,
+ data_interval=(self.default_time, self.default_time),
+ session=session,
+ )
+
+ tis = dagrun.get_task_instances(session=session)
+ assert tis, f"No task instances created for DAG {dagrun.dag_id}"
Review Comment:
We should replace `DAG` with `Dag` among the change
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/extra_links.py:
##########
@@ -87,6 +88,26 @@ def get_extra_links(
)
all_extra_links = {link_name: link_url or None for link_name, link_url in
sorted(all_extra_link_pairs)}
+ extra_link_records = (
+ session.query(XCom)
+ .filter(
+ XCom.dag_id == dag_id,
+ XCom.run_id == dag_run_id,
+ XCom.task_id == task_id,
+ XCom.map_index == map_index,
+ XCom.key.like("extra_link:%"),
Review Comment:
Would it be better to name the prefix as `_dynamic_link` and make it as a
constant in XCom module?
Since the _extra_link_ we introduce in this PR can be set in the runtime,
and #55194 is for solving the _static_ link ( already define in the operator
level ).
--
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]
