ashb commented on code in PR #55194: URL: https://github.com/apache/airflow/pull/55194#discussion_r2333992016
########## devel-common/src/tests_common/test_utils/mock_operators.py: ########## @@ -198,3 +198,22 @@ class GithubLink(BaseOperatorLink): def get_link(self, operator, *, ti_key): return "https://github.com/apache/airflow" + + +class InitCustomLink(BaseOperatorLink): + name = "Google" + + def get_link(self, operator, ti_key): + return "https://www.google.com" + + +class InitExtraLinksOperator(BaseOperator): + """Custom operator to test operator_extra_links defined in __init__.""" + + def __init__(self, value=None, **kwargs): + super().__init__(**kwargs) + self.value = value + self.operator_extra_links = (InitCustomLink(),) + + def execute(self, context): + pass Review Comment: This is only used in one place and isn't shared, so its best to put this in `airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_extra_links.py` ########## airflow-core/src/airflow/api_fastapi/core_api/routes/public/extra_links.py: ########## @@ -87,6 +87,29 @@ def get_extra_links( ) all_extra_links = {link_name: link_url or None for link_name, link_url in sorted(all_extra_link_pairs)} + from airflow.models.xcom import XComModel + + query = XComModel.get_many( + dag_ids=dag_id, + run_id=dag_run_id, + task_ids=task_id, + map_indexes=map_index, + ).filter(XComModel.key.like("_link_%")) Review Comment: ```suggestion ).filter(XComModel.key.like(r"\_link\_%")) ``` `_` is a special character in SQL LIKE expressions ########## airflow-core/src/airflow/api_fastapi/core_api/routes/public/extra_links.py: ########## @@ -87,6 +87,29 @@ def get_extra_links( ) all_extra_links = {link_name: link_url or None for link_name, link_url in sorted(all_extra_link_pairs)} + from airflow.models.xcom import XComModel + + query = XComModel.get_many( + dag_ids=dag_id, + run_id=dag_run_id, + task_ids=task_id, + map_indexes=map_index, + ).filter(XComModel.key.like("_link_%")) + + link_map = {} + if hasattr(task, "operator_extra_links"): + link_map = { + getattr(link, "xcom_key", link.name).removeprefix("_link_"): link.name + for link in getattr(task, "operator_extra_links", []) + } + + # Execute and iterate result rows (works on SQLA 1.4 and 2.0) + for row in session.execute(query).scalars(): + raw_class_name = row.key.removeprefix("_link_") + value = XComModel.deserialize_value(row) + display_name = link_map.get(raw_class_name, raw_class_name) Review Comment: I think we should probably consider something else here -- only being able to show the class name of the Link isn't really the best UX here. This likely needs a bigger change to how we and where we are storing things. ########## airflow-core/src/airflow/api_fastapi/core_api/routes/public/extra_links.py: ########## @@ -87,6 +87,29 @@ def get_extra_links( ) all_extra_links = {link_name: link_url or None for link_name, link_url in sorted(all_extra_link_pairs)} + from airflow.models.xcom import XComModel + + query = XComModel.get_many( + dag_ids=dag_id, + run_id=dag_run_id, + task_ids=task_id, + map_indexes=map_index, + ).filter(XComModel.key.like("_link_%")) + + link_map = {} + if hasattr(task, "operator_extra_links"): + link_map = { + getattr(link, "xcom_key", link.name).removeprefix("_link_"): link.name + for link in getattr(task, "operator_extra_links", []) + } + + # Execute and iterate result rows (works on SQLA 1.4 and 2.0) Review Comment: ```suggestion ``` Comment adds no value -- clearly it works, or we wouldn't have used it -- 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: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org