SameerMesiah97 commented on code in PR #60597:
URL: https://github.com/apache/airflow/pull/60597#discussion_r2695460817
##########
providers/amazon/src/airflow/providers/amazon/aws/operators/s3.py:
##########
@@ -337,6 +351,7 @@ def execute(self, context: Context):
self.source_version_id,
self.acl_policy,
self.meta_data_directive,
+ **copy_args,
)
Review Comment:
Passing the KMS parameters via **kwargs makes the new functionality
dependent on copy_object continuing to forward those parameters. The tests you
added do help protect against refactors of copy_object breaking this behavior,
but the contract itself isn’t explicitly represented in the public API. I
understand the motivation to avoid modifying the public API, but that argument
is more defensible for breaking changes. Adding optional parameters would be
non-breaking here and would make the contract more explicit. This is of course
just a suggestion as your tests prevent regressions.
--
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]
