Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve date/time hint handling for Redshift based on better testing #149

Merged
merged 6 commits into from
Dec 10, 2020

Conversation

vinceatbluelabs
Copy link
Contributor

Based on the new testing approach in #144, this PR adds component tests for date/time types while loading/exporting CSV files in Redshift databases.

Along the way some places for improvement were found!

Based on the new testing approach in #144, this PR adds component
tests for date/time types while loading/exporting CSV files in Redshift
databases.

Along the way some places for improvement were found!
Based on the new testing approach in #144, this PR adds component
tests for date/time types while loading/exporting CSV files in Redshift
databases.

Along the way some places for improvement were found!
fail_if_cant_handle_hint=True,
fail_if_row_invalid=True,
max_failure_rows=0)
self.assertIs(out['format'], Format.avro)
Copy link
Contributor Author

@vinceatbluelabs vinceatbluelabs Dec 9, 2020

Choose a reason for hiding this comment

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

The above was hanging out in tests/unit; I promoted it to tests/component and added the code below.

@vinceatbluelabs vinceatbluelabs marked this pull request as ready for review December 9, 2020 22:57
Copy link
Contributor

@cwegrzyn cwegrzyn left a comment

Choose a reason for hiding this comment

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

:shipit:

@vinceatbluelabs vinceatbluelabs merged commit f226ab4 into master Dec 10, 2020
@vinceatbluelabs vinceatbluelabs deleted the loosen_date_requirements_5 branch December 10, 2020 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants