potiuk commented on code in PR #70529:
URL: https://github.com/apache/airflow/pull/70529#discussion_r3681912045
##########
providers/google/src/airflow/providers/google/cloud/operators/cloud_storage_transfer_service.py:
##########
@@ -257,19 +257,19 @@ def __init__(
) -> None:
super().__init__(**kwargs)
self.body = body
- if isinstance(self.body, dict):
- self.body = deepcopy(body)
self.aws_conn_id = aws_conn_id
self.gcp_conn_id = gcp_conn_id
self.api_version = api_version
self.project_id = project_id
self.google_impersonation_chain = google_impersonation_chain
- self._validate_inputs()
def _validate_inputs(self) -> None:
TransferJobValidator(body=self.body).validate_body()
def execute(self, context: Context) -> dict:
+ if isinstance(self.body, dict):
+ self.body = deepcopy(self.body)
Review Comment:
The deepcopy is load-bearing and its purpose isn't obvious from here —
`TransferJobPreprocessor.process_body()` calls `_inject_aws_credentials()` and
`_reformat_schedule()`, which mutate the body **in place**. Without the copy,
the operator would write AWS credentials into the caller's own dict.
Two things worth doing:
1. **A one-line comment** saying why the copy exists. The original had the
copy sitting next to the assignment in `__init__` where the reader could at
least see it was defensive; now it's a bare reassignment in `execute` that
looks removable. This is precisely the kind of line someone "simplifies" away
later.
2. **A test for it.** The new test covers the validation-raises path, but
nothing asserts the behaviour the deepcopy is there for. Something like: build
the operator with a dict containing an `awsAccessKey`, run a successful
`execute`, and assert the dict you passed in is unchanged. Without that, the
copy could be dropped in a future refactor and the whole suite would stay green
while the operator started mutating user data.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
--
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]
