You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "ramonvermeulen (via GitHub)" <gi...@apache.org> on 2025/02/19 20:22:15 UTC
[PR] feat: draft implementation of exposing the GCP cloud logging url [airflow]
ramonvermeulen opened a new pull request, #46911:
URL: https://github.com/apache/airflow/pull/46911
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
* initial draft implementation of issue https://github.com/apache/airflow/issues/36963 to show as example, and open up the discussion
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: draft implementation of exposing the GCP cloud logging url [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2669677826
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:
- Pay attention to the quality of your code (ruff, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/dev/breeze/doc/README.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#coding-style-and-best-practices).
- Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "ramonvermeulen (via GitHub)" <gi...@apache.org>.
ramonvermeulen commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2830365324
Weirdly tests fail and seem to hit these airflow 3 >= lines while being ran with 2.9.3 and 2.10.5?:
https://github.com/apache/airflow/pull/46911/files#diff-27b491eef8e08f51786d7adcf1c491dbf6d6229cb5cdf91a499c8c11f73cfff1R30
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of exposing the GCP cloud logging url [airflow]
Posted by "ramonvermeulen (via GitHub)" <gi...@apache.org>.
ramonvermeulen commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2678320942
@potiuk @amoghrajesh @sunank200 @eladkal
Since I see that most of you are involved in decision making and recent contributions to the google-provider, and this is my first time contributing to airflow, I would love to hear your views on this feature.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal merged PR #46911:
URL: https://github.com/apache/airflow/pull/46911
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2854407823
> Back to your question: To fully solve the issue (after this PR) a solution needs to be implemented (if it is possible) to show the full cloud run job container logs directly in the Airflow operator.
So to allow dump of the remote job logs into the Airflow task like we do with AWS Glue?
https://github.com/apache/airflow/blob/3198aad1bfd7cbf8b3de56c482dc9d96e00d3a69/providers/amazon/src/airflow/providers/amazon/aws/operators/glue.py#L73
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2862072192
> Yes, something similar where the logs get fetched after completion via the logging API (or polled during job execution, but I think you will run into API limits even quicker). In the issue there is already [an example](https://github.com/apache/airflow/issues/36963#issuecomment-2669566660) provided, with a side note about the API limits.
in some of AWS operators that store logs in cloud watch there is the option to set fetch interval
https://github.com/apache/airflow/blob/cb295c351a016c0a10cab07f2a628b865cff3ca3/providers/amazon/src/airflow/providers/amazon/aws/operators/ecs.py#L375-L376
but lets not discuss this here. The reason I mentioned this is to ask if you can comment on the issue and explain what is left to implement after we merge this PR so it will be clear to the developer who takes the task
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2853475432
@ramonvermeulen i am going to merge this one once we get a green build.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "ramonvermeulen (via GitHub)" <gi...@apache.org>.
ramonvermeulen commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2853653511
> > Implementation to (partly) solve issue CloudRunExecuteJobOperator - logs from inside the container are not shown in Airflow #36963
>
> Can you clarify what else would be needed to fully solve the issue after this PR is merged? Should we close the original issue and open a new one on the specific task?
What I mean with "partly" solves the issue is the following:
- The issue is about a full log integration between the logs of the cloud run jobs and Airflow. So that you can really see the full cloud run job logs directly in Airflow. However, this is really hard to implement and you will run into quota issues on the GCP logging API quite soon.
Therefore I created this PR, which partly solves the issue. Instead of integrating the logs directly in Airflow, I added an extra link, so that you can click on the Airflow UI and get redirected to the logs in GCP which is quite handy IMO.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "ramonvermeulen (via GitHub)" <gi...@apache.org>.
ramonvermeulen commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2834093542
> > Weirdly tests fail and seem to hit these airflow 3 >= lines while being ran with 2.9.3 and 2.10.5?:
> > https://github.com/apache/airflow/pull/46911/files#diff-27b491eef8e08f51786d7adcf1c491dbf6d6229cb5cdf91a499c8c11f73cfff1R30
>
> Not weirdly - airflow.sdk does not existi in Airflow 2 and the test import it - so it fail, there needs to be compatibility code in the test to run also in Airflow 2.
Whoops, was fixated on the other test that already had the compatibility code.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "ramonvermeulen (via GitHub)" <gi...@apache.org>.
ramonvermeulen commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2862130531
> > Yes, something similar where the logs get fetched after completion via the logging API (or polled during job execution, but I think you will run into API limits even quicker). In the issue there is already [an example](https://github.com/apache/airflow/issues/36963#issuecomment-2669566660) provided, with a side note about the API limits.
>
> in some of AWS operators that store logs in cloud watch there is the option to set fetch interval
>
> https://github.com/apache/airflow/blob/cb295c351a016c0a10cab07f2a628b865cff3ca3/providers/amazon/src/airflow/providers/amazon/aws/operators/ecs.py#L375-L376
>
> but lets not discuss this here. The reason I mentioned this is to ask if you can comment on the issue and explain what is left to implement after we merge this PR so it will be clear to the developer who takes the task
Sure, that makes sense! I will give an update in the issue after the PR is merged 👍
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2853528123
> Implementation to (partly) solve issue CloudRunExecuteJobOperator - logs from inside the container are not shown in Airflow #36963
Can you clarify what else would be needed to fully solve the issue after this PR is merged? Should we close the original issue and open a new one on the specific task?
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "ramonvermeulen (via GitHub)" <gi...@apache.org>.
ramonvermeulen commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2741293529
Just tested, and I can confirm the fix solved my issue :)
<img width="1005" alt="image" src="https://github.com/user-attachments/assets/b33ba5fb-a8eb-4ded-bc39-432148b55aa2" />
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "ramonvermeulen (via GitHub)" <gi...@apache.org>.
ramonvermeulen commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2742567375
> Code looks good and I am happy to approve once you have added the test I requested
Just updated the PR with a test for `get_link`, it works slightly different than the example you provided because this link comes directly from the API, and is not constructed with a format/template string like most of the extra link implementations.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #46911:
URL: https://github.com/apache/airflow/pull/46911#discussion_r2006972595
##########
providers/google/tests/unit/google/cloud/links/test_cloud_run.py:
##########
@@ -0,0 +1,47 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from unittest import mock
+
+from airflow.providers.google.cloud.links.cloud_run import CloudRunJobLoggingLink
+
+TEST_LOG_URI = (
+ "https://console.cloud.google.com/run/jobs/logs?project=test-project®ion=test-region&job=test-job"
+)
+
+
+class TestCloudRunJobLoggingLink:
+ def test_class_attributes(self):
+ assert CloudRunJobLoggingLink.key == "log_uri"
+ assert CloudRunJobLoggingLink.name == "Cloud Run Job Logging"
+
+ def test_persist(self):
+ mock_context, mock_task_instance = mock.MagicMock(), mock.MagicMock()
+
+ CloudRunJobLoggingLink.persist(
+ context=mock_context,
+ task_instance=mock_task_instance,
+ log_uri=TEST_LOG_URI,
+ )
+
+ mock_task_instance.xcom_push.assert_called_once_with(
+ mock_context,
+ key=CloudRunJobLoggingLink.key,
+ value=TEST_LOG_URI,
+ )
Review Comment:
Can we also add a test that does a `get_link`?
Similar to:
```
class TestTranslationLegacyDatasetLink:
@pytest.mark.db_test
def test_get_link(self, create_task_instance_of_operator, session, mock_supervisor_comms):
expected_url = f"{TRANSLATION_BASE_LINK}/locations/{GCP_LOCATION}/datasets/{DATASET}/sentences?project={GCP_PROJECT_ID}"
link = TranslationLegacyDatasetLink()
ti = create_task_instance_of_operator(
AutoMLCreateDatasetOperator,
dag_id="test_legacy_dataset_link_dag",
task_id="test_legacy_dataset_link_task",
dataset=DATASET,
location=GCP_LOCATION,
)
session.add(ti)
session.commit()
link.persist(context={"ti": ti}, task_instance=ti.task, dataset_id=DATASET, project_id=GCP_PROJECT_ID)
if AIRFLOW_V_3_0_PLUS and mock_supervisor_comms:
mock_supervisor_comms.get_message.return_value = XComResult(
key="key",
value={"location": ti.task.location, "dataset_id": DATASET, "project_id": GCP_PROJECT_ID},
)
actual_url = link.get_link(operator=ti.task, ti_key=ti.key)
assert actual_url == expected_url
```
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of exposing the GCP cloud logging url [airflow]
Posted by "ramonvermeulen (via GitHub)" <gi...@apache.org>.
ramonvermeulen commented on code in PR #46911:
URL: https://github.com/apache/airflow/pull/46911#discussion_r1967674362
##########
providers/google/src/airflow/providers/google/cloud/operators/cloud_run.py:
##########
@@ -301,6 +306,9 @@ def execute(self, context: Context):
if self.operation is None:
raise AirflowException("Operation is None")
+ if self.expose_logging_url:
+ self.log.info("GCP Console Logging URL: %s ", self.operation.metadata.log_uri)
Review Comment:
The use-case is that there is a direct link available from within the Airflow UI/logs, to show the actual cloud run job container logs of that specific execution. In the issue there was also discussion going on, to have an implementation that polls the GCP Logging API, to make the logs available line by line in the Airflow UI. However, there is quite a strict default quota on the cloud logging API, for me the implementation of just having a direct link available to the container logs in the GCP console will already help a lot, therefore I opened this PR.
Depends, but normally when you're already signed in to the GCP console, you do not need to sign-in again for (I think 24 hours?).
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of exposing the GCP cloud logging url [airflow]
Posted by "ramonvermeulen (via GitHub)" <gi...@apache.org>.
ramonvermeulen commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2729945061
@ashb
Since I am back from holidays, I tried to do the implementation via `OperatorExtraLinks`. However, I have a hard time testing the set-up. I recently pulled in the latest changes from the upstream airflow repository, and when I run `breeze start-airflow` it uses Airflow 3 now.
With Airflow 3, I get the following error on the base class I am using:
`Direct database access via the ORM is not allowed in Airflow 3.0`
Line causing the error: https://github.com/apache/airflow/blob/2bc6f09ea3865cb0c0479efa88cab5256e96f7e3/providers/google/src/airflow/providers/google/cloud/links/base.py#L54
If this is something that needs to be solved for Airflow 3 for all the Google Provider Links anyways, it would be nice if I can add that to the PR. However, since this is my first PR I do not have a great understanding of the airflow codebase (yet). Not sure what should be used instead of the ORM in Airflow 3> to access the XCom from an OperatorExtraLink?
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "ramonvermeulen (via GitHub)" <gi...@apache.org>.
ramonvermeulen commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2731964728
Managed to get rid of the above error, and again pulled in the latest upstream.
However, I notice there is an error when the web app tries to fetch the extra link and I have the feeling it might be related to recent changes:
https://github.com/apache/airflow/blob/df584d3c8f397dbfec33f383292a9ca7c05dcc07/task-sdk/src/airflow/sdk/definitions/baseoperatorlink.py#L48 in
https://github.com/apache/airflow/commit/fe5a2ea7a0b57901bb6239d666b875f6c71cd7e8
Line causing the error https://github.com/apache/airflow/blob/0010596c56f7f8b9607e8d98622fb7c1b7edff71/airflow/models/xcom.py#L338
<details>
<summary>Stacktrace</summary>
```
Traceback (most recent call last):
File "/usr/local/lib/python3.9/site-packages/uvicorn/protocols/http/httptools_impl.py", line 409, in run_asgi
result = await app( # type: ignore[func-returns-value]
File "/usr/local/lib/python3.9/site-packages/uvicorn/middleware/proxy_headers.py", line 60, in __call__
return await self.app(scope, receive, send)
File "/usr/local/lib/python3.9/site-packages/fastapi/applications.py", line 1054, in __call__
await super().__call__(scope, receive, send)
File "/usr/local/lib/python3.9/site-packages/starlette/applications.py", line 112, in __call__
await self.middleware_stack(scope, receive, send)
File "/usr/local/lib/python3.9/site-packages/starlette/middleware/errors.py", line 187, in __call__
raise exc
File "/usr/local/lib/python3.9/site-packages/starlette/middleware/errors.py", line 165, in __call__
await self.app(scope, receive, _send)
File "/usr/local/lib/python3.9/site-packages/starlette/middleware/gzip.py", line 29, in __call__
await responder(scope, receive, send)
File "/usr/local/lib/python3.9/site-packages/starlette/middleware/gzip.py", line 126, in __call__
await super().__call__(scope, receive, send)
File "/usr/local/lib/python3.9/site-packages/starlette/middleware/gzip.py", line 46, in __call__
await self.app(scope, receive, self.send_with_compression)
File "/usr/local/lib/python3.9/site-packages/starlette/middleware/cors.py", line 85, in __call__
await self.app(scope, receive, send)
File "/usr/local/lib/python3.9/site-packages/starlette/middleware/base.py", line 178, in __call__
recv_stream.close()
File "/usr/local/lib/python3.9/contextlib.py", line 137, in __exit__
self.gen.throw(typ, value, traceback)
File "/usr/local/lib/python3.9/site-packages/starlette/_utils.py", line 82, in collapse_excgroups
raise exc
File "/usr/local/lib/python3.9/site-packages/starlette/middleware/base.py", line 175, in __call__
response = await self.dispatch_func(request, call_next)
File "/opt/airflow/airflow/api_fastapi/core_api/middleware.py", line 28, in dispatch
response = await call_next(request)
File "/usr/local/lib/python3.9/site-packages/starlette/middleware/base.py", line 153, in call_next
raise app_exc
File "/usr/local/lib/python3.9/site-packages/starlette/middleware/base.py", line 140, in coro
await self.app(scope, receive_or_disconnect, send_no_error)
File "/usr/local/lib/python3.9/site-packages/starlette/middleware/exceptions.py", line 62, in __call__
await wrap_app_handling_exceptions(self.app, conn)(scope, receive, send)
File "/usr/local/lib/python3.9/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
raise exc
File "/usr/local/lib/python3.9/site-packages/starlette/_exception_handler.py", line 42, in wrapped_app
await app(scope, receive, sender)
File "/usr/local/lib/python3.9/site-packages/starlette/routing.py", line 714, in __call__
await self.middleware_stack(scope, receive, send)
File "/usr/local/lib/python3.9/site-packages/starlette/routing.py", line 734, in app
await route.handle(scope, receive, send)
File "/usr/local/lib/python3.9/site-packages/starlette/routing.py", line 288, in handle
await self.app(scope, receive, send)
File "/usr/local/lib/python3.9/site-packages/starlette/routing.py", line 76, in app
await wrap_app_handling_exceptions(app, request)(scope, receive, send)
File "/usr/local/lib/python3.9/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
raise exc
File "/usr/local/lib/python3.9/site-packages/starlette/_exception_handler.py", line 42, in wrapped_app
await app(scope, receive, sender)
File "/usr/local/lib/python3.9/site-packages/starlette/routing.py", line 73, in app
response = await f(request)
File "/usr/local/lib/python3.9/site-packages/fastapi/routing.py", line 301, in app
raw_response = await run_endpoint_function(
File "/usr/local/lib/python3.9/site-packages/fastapi/routing.py", line 214, in run_endpoint_function
return await run_in_threadpool(dependant.call, **values)
File "/usr/local/lib/python3.9/site-packages/starlette/concurrency.py", line 37, in run_in_threadpool
return await anyio.to_thread.run_sync(func)
File "/usr/local/lib/python3.9/site-packages/anyio/to_thread.py", line 56, in run_sync
return await get_async_backend().run_sync_in_worker_thread(
File "/usr/local/lib/python3.9/site-packages/anyio/_backends/_asyncio.py", line 2470, in run_sync_in_worker_thread
return await future
File "/usr/local/lib/python3.9/site-packages/anyio/_backends/_asyncio.py", line 967, in run
result = context.run(func, *args)
File "/opt/airflow/airflow/api_fastapi/core_api/routes/public/extra_links.py", line 85, in get_extra_links
all_extra_links = {link_name: link_url or None for link_name, link_url in sorted(all_extra_link_pairs)}
File "/opt/airflow/airflow/api_fastapi/core_api/routes/public/extra_links.py", line 83, in <genexpr>
(link_name, task.get_extra_links(ti, link_name)) for link_name in task.extra_links
File "/opt/airflow/airflow/serialization/serialized_objects.py", line 1220, in get_extra_links
return link.get_link(self.unmap(None), ti_key=ti.key)
File "/opt/airflow/airflow/serialization/serialized_objects.py", line 2036, in get_link
return XComModel.deserialize_value(value)
File "/opt/airflow/airflow/models/xcom.py", line 338, in deserialize_value
return json.loads(result.value, cls=XComDecoder)
File "/usr/local/lib/python3.9/json/__init__.py", line 359, in loads
return cls(**kw).decode(s)
File "/usr/local/lib/python3.9/json/decoder.py", line 337, in decode
obj, end = self.raw_decode(s, idx=_w(s, 0).end())
File "/usr/local/lib/python3.9/json/decoder.py", line 355, in raw_decode
raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
```
</details>
Error has to do with the `f"_link_{self.__class__.__name__}"` being retrieved from XCom, however not being json serializable (e.g. it is just a string with the URI and not valid json). I tested existing google operators on the latest main such as `BigQueryCreateEmptyTableOperator` with `BigQueryTableLink` and I get exactly the same error so I have the feeling it is not related to my specific link implementation.
To give an example, this is how the XCom looks like for the CloudRunExecuteJobOperator:
And for the BigQueryCreateTableOperator that I tested:
<img width="1120" alt="2" src="https://github.com/user-attachments/assets/a4a6c4b7-8ca8-4595-8b97-5bb8ba852079" />
Both seem to contain a valid link, however the API server throws a 500 with the jsondecode error.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "ramonvermeulen (via GitHub)" <gi...@apache.org>.
ramonvermeulen commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2739418973
This https://github.com/apache/airflow/pull/47961
Should resolve my issue, will pull in upstream and test this evening :)
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2831086554
> Weirdly tests fail and seem to hit these airflow 3 >= lines while being ran with 2.9.3 and 2.10.5?:
>
> https://github.com/apache/airflow/pull/46911/files#diff-27b491eef8e08f51786d7adcf1c491dbf6d6229cb5cdf91a499c8c11f73cfff1R30
Not weirdly - airflow.sdk does not existi in Airflow 2 and the test import it - so it fail, there needs to be compatibility code in the test to run also in Airflow 2.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #46911:
URL: https://github.com/apache/airflow/pull/46911#discussion_r2062947486
##########
providers/google/tests/unit/google/cloud/links/test_cloud_run.py:
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from unittest import mock
+
+import pytest
+
+from airflow.providers.google.cloud.links.cloud_run import CloudRunJobLoggingLink
+from airflow.providers.google.cloud.operators.cloud_run import CloudRunExecuteJobOperator
+from airflow.providers.google.version_compat import AIRFLOW_V_3_0_PLUS
+from airflow.sdk.execution_time.comms import XComResult
Review Comment:
The tests are failing cos of this line, add it to AF3 check
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "ramonvermeulen (via GitHub)" <gi...@apache.org>.
ramonvermeulen commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2834076600
> > Weirdly tests fail and seem to hit these airflow 3 >= lines while being ran with 2.9.3 and 2.10.5?:
> > https://github.com/apache/airflow/pull/46911/files#diff-27b491eef8e08f51786d7adcf1c491dbf6d6229cb5cdf91a499c8c11f73cfff1R30
>
> Not weirdly - airflow.sdk does not existi in Airflow 2 and the test import it - so it fail, there needs to be compatibility code in the test to run also in Airflow 2.
Isn't that where this if/else is for?:
```
if AIRFLOW_V_3_0_PLUS:
from airflow.sdk.execution_time.xcom import XCom
else:
from airflow.models.xcom import XCom # type: ignore[no-redef]
```
e.g. within an Airflow 3 >= runtime `AIRFLOW_V_3_0_PLUS` evaluates as `False` so the `from airflow.sdk.execution_time.xcom import XCom` import statement gets skipped? Just like how `if typing.TYPE_CHECKING:` works, at runtime it doesn't get importend since it always evaluates `False` but helps for IDE typing support.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2862073292
@ramonvermeulen can you rebase and resolve conflicts?
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of BaseOperatorLink for cloud run jobs to expose the GCP cloud logging url [airflow]
Posted by "ramonvermeulen (via GitHub)" <gi...@apache.org>.
ramonvermeulen commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2854832124
> > Back to your question: To fully solve the issue (after this PR) a solution needs to be implemented (if it is possible) to show the full cloud run job container logs directly in the Airflow operator.
>
> So to allow dump of the remote job logs into the Airflow task like we do with AWS Glue?
>
> https://github.com/apache/airflow/blob/3198aad1bfd7cbf8b3de56c482dc9d96e00d3a69/providers/amazon/src/airflow/providers/amazon/aws/operators/glue.py#L73
Yes, something similar where the logs get fetched after completion via the logging API. In the issue there is already [an example](https://github.com/apache/airflow/issues/36963#issuecomment-2669566660), with a side note about the API quotas.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of exposing the GCP cloud logging url [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46911:
URL: https://github.com/apache/airflow/pull/46911#discussion_r1967654358
##########
providers/google/src/airflow/providers/google/cloud/operators/cloud_run.py:
##########
@@ -288,6 +292,7 @@ def __init__(
self.polling_period_seconds = polling_period_seconds
self.timeout_seconds = timeout_seconds
self.deferrable = deferrable
+ self.expose_logging_url = expose_logging_url
Review Comment:
```suggestion
self.expose_logging_url = expose_logging_url if expose_logging_url else False
```
##########
providers/google/src/airflow/providers/google/cloud/operators/cloud_run.py:
##########
@@ -275,6 +278,7 @@ def __init__(
timeout_seconds: float | None = None,
gcp_conn_id: str = "google_cloud_default",
impersonation_chain: str | Sequence[str] | None = None,
+ expose_logging_url: bool = False,
Review Comment:
```suggestion
expose_logging_url: bool | None= False,
```
##########
providers/google/src/airflow/providers/google/cloud/operators/cloud_run.py:
##########
@@ -301,6 +306,9 @@ def execute(self, context: Context):
if self.operation is None:
raise AirflowException("Operation is None")
+ if self.expose_logging_url:
+ self.log.info("GCP Console Logging URL: %s ", self.operation.metadata.log_uri)
Review Comment:
When this is enabled, do we need to log in to the GCP console only once, or should we also log in after the job is deferred and completed? What is the use case here?
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of exposing the GCP cloud logging url [airflow]
Posted by "ramonvermeulen (via GitHub)" <gi...@apache.org>.
ramonvermeulen commented on PR #46911:
URL: https://github.com/apache/airflow/pull/46911#issuecomment-2691212684
> Rather than just putting this URL in the log message, please look at the "OperatorExtraLinks" concept -- you can make this show up as a button in the Airflow UI.
>
> (They way I would suggest wiring that up is to have the operator push the url to XCom, and the OperatorLink can then pull the URL out of xcom)
Thanks, wasn't aware of that functionality. Will see if I can change the implementation after the weekend to make use of `OperatorExtraLinks` 👍
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: implementation of exposing the GCP cloud logging url [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46911:
URL: https://github.com/apache/airflow/pull/46911#discussion_r1967660112
##########
providers/google/src/airflow/providers/google/cloud/operators/cloud_run.py:
##########
@@ -301,6 +306,9 @@ def execute(self, context: Context):
if self.operation is None:
raise AirflowException("Operation is None")
+ if self.expose_logging_url:
+ self.log.info("GCP Console Logging URL: %s ", self.operation.metadata.log_uri)
Review Comment:
When this is enabled, do we need to log from GCP console only once, or should we also log in after the job is deferred and completed? What is the use case here?
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org