Skip to content

Conversation

@vincbeck
Copy link
Contributor

@vincbeck vincbeck commented May 17, 2023

The system test example_dynamodb_to_s3 is currently failing for multiple reasons:

  • The point in time recovery must be enabled in the table
  • Once enabled, you need to get a valid export time. You can get such information by calling describe_continuous_backups API.

See related: #31307


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
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.

@vincbeck
Copy link
Contributor Author

cc @utkarsharma2

TableName=table_name,
)

return r["ContinuousBackupsDescription"]["PointInTimeRecoveryDescription"]["EarliestRestorableDateTime"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, that's nicer than my created time idea. Nice find.

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good! thanks for taking care of this.

def _export_entire_data(self):
"""Export all data from the table."""
table = self.hook.get_conn().Table(self.dynamodb_table_name)
table = self.hook.conn.Table(self.dynamodb_table_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks.

Comment on lines +100 to +107
"dynamodb_table_name",
"s3_bucket_name",
"file_size",
"dynamodb_scan_kwargs",
"s3_key_prefix",
"dynamodb_table_name",
"process_func",
"export_time",
"export_format",
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks.

@ferruzzi
Copy link
Contributor

One of your unit tests are failing, bit otherwise LGTM. Some nice nitpick catches in there as well, thanks for those.

@utkarsharma2
Copy link
Contributor

@vincbeck Thanks for handling the timezone scenario as well. LGTM :)

@potiuk
Copy link
Member

potiuk commented May 17, 2023

(Approved - pending the unit-test fix :)

@vincbeck
Copy link
Contributor Author

All green :)

@potiuk potiuk merged commit 0b3b670 into apache:main May 17, 2023
@vincbeck vincbeck deleted the vincbeck/system-test/dynamodb branch May 17, 2023 17:57
@vincbeck
Copy link
Contributor Author

:D

Screenshot 2023-05-17 at 4 56 57 PM

@potiuk
Copy link
Member

potiuk commented May 17, 2023

🎉 🎉 🎉 🎉 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants