vincbeck commented on code in PR #41304:
URL: https://github.com/apache/airflow/pull/41304#discussion_r1713848627
##########
airflow/providers/amazon/aws/transfers/dynamodb_to_s3.py:
##########
@@ -148,29 +174,58 @@ def hook(self):
return DynamoDBHook(aws_conn_id=self.source_aws_conn_id)
def execute(self, context: Context) -> None:
- if self.export_time:
+ # There are 2 separate export to point in time configuration:
+ # 1. Full export, which takes the export_time arg.
+ # 2. Incremental export, which takes the incremental_export_... args
+ # Hence export time could not be used as the proper indicator for the
`_export_table_to_point_in_time`
+ # function. This change introduces a new boolean, as the indicator for
whether the operator scans
+ # and export entire data or using the point in time functionality.
+ if self.point_in_time_export:
Review Comment:
This change is breaking change. By introducing this change, you'll break
DAGs of users using this operator. If a user uses this operator with
`export_time` provided, you'll break their experience. Also, I would suggesting
making this change in a different way. Why dont you expose a new parameter on
the operator level `export_table_to_point_in_time_kwargs` which let the user
define whatever extra parameter they want to `export_table_to_point_in_time`
call? As a user you would be able to achieve what you are trying to do and the
change would be way simpler
##########
airflow/providers/amazon/aws/transfers/dynamodb_to_s3.py:
##########
@@ -84,15 +84,29 @@ class DynamoDBToS3Operator(AwsToAwsBaseOperator):
:param dynamodb_table_name: Dynamodb table to replicate data from
:param s3_bucket_name: S3 bucket to replicate data to
+ :param s3_bucket_owner: The ID of the Amazon Web Services account that
owns the bucket the export will be stored in.
+ (NOTE: S3BucketOwner is a required parameter when exporting to a S3 bucket
in another account.)
Review Comment:
```suggestion
(NOTE:``s3_bucket_owner`` is a required parameter when exporting to a S3
bucket in another account.)
```
##########
airflow/providers/amazon/aws/transfers/dynamodb_to_s3.py:
##########
@@ -84,15 +84,29 @@ class DynamoDBToS3Operator(AwsToAwsBaseOperator):
:param dynamodb_table_name: Dynamodb table to replicate data from
:param s3_bucket_name: S3 bucket to replicate data to
+ :param s3_bucket_owner: The ID of the Amazon Web Services account that
owns the bucket the export will be stored in.
+ (NOTE: S3BucketOwner is a required parameter when exporting to a S3 bucket
in another account.)
Review Comment:
```suggestion
(NOTE:``s3_bucket_owner`` is a required parameter when exporting to a S3
bucket in another account.)
```
--
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]
