Skip to content

Log the HTTP response in HttpToGCSOperator when log_response is set - #72952

Merged
potiuk merged 2 commits into
apache:mainfrom
bingqin2:http-to-gcs-log-response
Sep 20, 2026
Merged

potiuk merged 2 commits into
apache:mainfrom
bingqin2:http-to-gcs-log-response

Conversation

@bingqin2

Copy link
Copy Markdown
Contributor

HttpToGCSOperator documents log_response ("Log the response (default: False)") and stores it, but execute() never reads it, so the response body is never logged no matter what the user sets. This logs response.text after the request when the flag is set, the same way HttpOperator does.

Changes

  • providers/google/src/airflow/providers/google/cloud/transfers/http_to_gcs.py: log the response text when log_response is true
  • providers/google/tests/unit/google/cloud/transfers/test_http_to_gcs.py: test_execute_logs_response_only_when_requested, parametrized over both values

Testing

  • providers/google: tests/unit/google/cloud/transfers/test_http_to_gcs.py
  • mypy on the changed module, prek hooks on the changed files

Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: Claude Code (Claude Fable 5.1) following the guidelines. I reviewed and understand all changes; the tests were run locally as listed above.


🤖 Generated with Claude Code

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a correct, minimal fix for a genuine silent no-op — log_response was accepted, documented and stored by HttpToGCSOperator but never read in execute(), and the two added lines mirror what HttpOperator.process_response() already does. The parametrized regression test fails without the fix and no existing test or behaviour was touched.

Smaller observations

  • One test-style nit inline about caplog.text.
  • The commit carries a Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> trailer. Per the repo guidelines, "Never list an agent as a commit co-author" — the checked Gen-AI disclosure plus the Generated-by: line in the PR body is the sanctioned form. I'm dropping that trailer from the squash commit message, so nothing to do on your side.

This review was drafted by an AI-assisted tool and
confirmed by an Apache Airflow maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.

More on how Apache Airflow handles maintainer review:
contributing-docs/05_pull_requests.rst.

Comment thread providers/google/tests/unit/google/cloud/transfers/test_http_to_gcs.py Outdated
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@potiuk
potiuk force-pushed the http-to-gcs-log-response branch from dc1e7d8 to 4496c3d Compare September 20, 2026 17:17
caplog.text is a legacy string-matching API the testing standards ask us to
avoid, and the documented "in caplog" replacement is not usable here: the
google provider supports Airflow 2.11+, and in the compat jobs caplog falls
back to the stock pytest fixture, which has no __contains__. Patching
task.log.info follows the http provider's own test for this feature and drops
the log-capture machinery entirely.

Generated-by: Claude Opus 5
@potiuk
potiuk merged commit a7132cc into apache:main Sep 20, 2026
86 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:google Google (including GCP) related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants