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

Add json as allowable file type to copy_s3 #844

Merged
merged 32 commits into from
Sep 7, 2023
Merged

Conversation

KasiaHinkson
Copy link
Contributor

Add json as allowable file type to copy_s3. Since it's converting to a Parsons table, should be no downstream effects.

shaunagm
shaunagm previously approved these changes Jun 22, 2023
Copy link
Collaborator

@shaunagm shaunagm left a comment

Choose a reason for hiding this comment

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

This looks good to me, though I haven't tested it. You should merge if/when you feel it's ready.

@KasiaHinkson
Copy link
Contributor Author

My test case failed because it's line delimited JSON, which Redshift can't handle because Redshift is a big baby, so I'll test this as soon as I can find a better test case

@shaunagm shaunagm dismissed their stale review July 24, 2023 17:33

this needs changes and so I want to get the state of the PR correct

@KasiaHinkson
Copy link
Contributor Author

Ok, this branch has now been tested VERY thoroughly, our Hustle sync is actively running on this branch, so I'm confident it's ready for review

Copy link
Contributor

@sharinetmc sharinetmc left a comment

Choose a reason for hiding this comment

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

LGTM!

@sharinetmc sharinetmc merged commit fce51b0 into main Sep 7, 2023
5 checks passed
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.

3 participants