amoghrajesh commented on code in PR #46613:
URL: https://github.com/apache/airflow/pull/46613#discussion_r1954725637
##########
tests_common/pytest_plugin.py:
##########
@@ -1606,6 +1607,18 @@ def _disable_redact(request: pytest.FixtureRequest,
mocker):
return
[email protected](autouse=True)
+def _mock_plugins(request: pytest.FixtureRequest):
+ """Disable redacted text in tests, except specific."""
+ if mark := next(request.node.iter_markers("mock_plugin_manager"), None):
+ from tests_common.test_utils.mock_plugins import mock_plugin_manager
+
+ with mock_plugin_manager(**mark.kwargs):
+ yield
+ return
+ yield
Review Comment:
This is important to introduce because I was running into a scenario where
the `mock_plugin_manager` as a wrapper was running before the dag was
serialising, leading to some issues like the test plugins loading when I didnt
wish for them too.
##########
tests/api_fastapi/core_api/routes/public/test_extra_links.py:
##########
@@ -43,7 +39,33 @@
pytestmark = pytest.mark.db_test
-class TestExtraLinks:
+class GoogleLink(BaseOperatorLink):
+ name = "Google"
+
+ def get_link(self, operator, ti_key):
+ return "https://www.google.com";
Review Comment:
New style, not date time.
##########
tests/api_fastapi/core_api/routes/public/test_extra_links.py:
##########
@@ -158,15 +181,33 @@ def test_should_respond_200_missing_xcom(self,
test_client):
assert response.status_code == 200
assert response.json() == {"Google Custom": None}
- @mock_plugin_manager(plugins=[])
- def test_should_respond_200_multiple_links(self, test_client):
+ def test_should_respond_200_multiple_links(self, test_client, session):
XCom.set(
key="search_query",
value=["TEST_LINK_VALUE_1", "TEST_LINK_VALUE_2"],
task_id=self.task_multiple_links,
dag_id=self.dag.dag_id,
run_id=self.dag_run_id,
+ session=session,
+ )
+ XCom.set(
+ key="bigquery_1",
+
value="https://console.cloud.google.com/bigquery?j=TEST_LINK_VALUE_1";,
+ task_id=self.task_multiple_links,
+ dag_id=self.dag_id,
+ run_id=self.dag_run_id,
+ session=session,
+ )
+ XCom.set(
+ key="bigquery_2",
+
value="https://console.cloud.google.com/bigquery?j=TEST_LINK_VALUE_2";,
+ task_id=self.task_multiple_links,
+ dag_id=self.dag_id,
+ run_id=self.dag_run_id,
+ session=session,
Review Comment:
Actually setting these, so it can be fetched from xcom instead of mocking.
##########
tests/api_fastapi/core_api/routes/public/test_extra_links.py:
##########
@@ -186,48 +226,19 @@ def
test_should_respond_200_multiple_links_missing_xcom(self, test_client):
assert response.status_code == 200
assert response.json() == {"BigQuery Console #1": None, "BigQuery
Console #2": None}
+ @pytest.mark.mock_plugin_manager(plugins=[AirflowPluginWithOperatorLinks])
Review Comment:
This fixture will override global one
##########
tests/api_fastapi/core_api/routes/public/test_extra_links.py:
##########
@@ -43,7 +39,33 @@
pytestmark = pytest.mark.db_test
-class TestExtraLinks:
+class GoogleLink(BaseOperatorLink):
+ name = "Google"
+
+ def get_link(self, operator, ti_key):
+ return "https://www.google.com";
+
+
+class S3LogLink(BaseOperatorLink):
+ name = "S3"
+ operators = [CustomOperator]
+
+ def get_link(self, operator, ti_key):
+ return
f"https://s3.amazonaws.com/airflow-logs/{operator.dag_id}/{operator.task_id}/";
+
+
+class AirflowPluginWithOperatorLinks(AirflowPlugin):
+ name = "test_plugin"
+ global_operator_extra_links = [
+ GoogleLink(),
+ ]
+ operator_extra_links = [
+ S3LogLink(),
+ ]
+
+
[email protected]_plugin_manager(plugins=[])
Review Comment:
This is one such example of that scenario mentioned above.
##########
airflow/models/abstractoperator.py:
##########
@@ -157,64 +153,6 @@ def priority_weight_total(self) -> int:
)
)
- @cached_property
- def operator_extra_link_dict(self) -> dict[str, Any]:
- """Returns dictionary of all extra links for the operator."""
- op_extra_links_from_plugin: dict[str, Any] = {}
- from airflow import plugins_manager
-
- plugins_manager.initialize_extra_operators_links_plugins()
- if plugins_manager.operator_extra_links is None:
- raise AirflowException("Can't load operators")
- for ope in plugins_manager.operator_extra_links:
- if ope.operators and self.operator_class in ope.operators:
- op_extra_links_from_plugin.update({ope.name: ope})
-
- operator_extra_links_all = {link.name: link for link in
self.operator_extra_links}
- # Extra links defined in Plugins overrides operator links defined in
operator
- operator_extra_links_all.update(op_extra_links_from_plugin)
-
- return operator_extra_links_all
-
- @cached_property
- def global_operator_extra_link_dict(self) -> dict[str, Any]:
- """Returns dictionary of all global extra links."""
- from airflow import plugins_manager
-
- plugins_manager.initialize_extra_operators_links_plugins()
- if plugins_manager.global_operator_extra_links is None:
- raise AirflowException("Can't load operators")
- return {link.name: link for link in
plugins_manager.global_operator_extra_links}
-
- @cached_property
- def extra_links(self) -> list[str]:
- return
sorted(set(self.operator_extra_link_dict).union(self.global_operator_extra_link_dict))
-
- def get_extra_links(self, ti: TaskInstance, link_name: str) -> str | None:
- """
- For an operator, gets the URLs that the ``extra_links`` entry points
to.
-
- :meta private:
-
- :raise ValueError: The error message of a ValueError will be passed on
through to
- the fronted to show up as a tooltip on the disabled link.
- :param ti: The TaskInstance for the URL being searched for.
- :param link_name: The name of the link we're looking for the URL for.
Should be
- one of the options specified in ``extra_links``.
- """
- link: BaseOperatorLink | None =
self.operator_extra_link_dict.get(link_name)
- if not link:
- link = self.global_operator_extra_link_dict.get(link_name)
- if not link:
- return None
-
- parameters = inspect.signature(link.get_link).parameters
- old_signature = all(name != "ti_key" for name, p in parameters.items()
if p.kind != p.VAR_KEYWORD)
-
- if old_signature:
- return link.get_link(self.unmap(None), ti.dag_run.logical_date) #
type: ignore[misc]
- return link.get_link(self.unmap(None), ti_key=ti.key)
Review Comment:
Webserver should not be dealing with real abstarctoperator objects. It does
and should deal with SerialisedBaseOperator objects. Hence moving this there
and nuking this one!
--
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]
