-
Notifications
You must be signed in to change notification settings - Fork 94
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
feat: support larger seed files #161
Conversation
Signed-off-by: Henri Blancke <blanckehenri@gmail.com>
Does seeds still support passing the seed file as csv? |
Signed-off-by: Henri Blancke <blanckehenri@gmail.com>
Yep, files are still being passed as |
Signed-off-by: Henri Blancke <blanckehenri@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I left some small nits.
Signed-off-by: Henri Blancke <blanckehenri@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
Signed-off-by: Henri Blancke <blanckehenri@gmail.com>
Signed-off-by: Henri Blancke <blanckehenri@gmail.com>
@henriblancke please rebase with main - then I will test and review this feature. |
Signed-off-by: Henri Blancke <blanckehenri@gmail.com>
@nicor88 rebased ✅ |
Signed-off-by: Henri Blancke <blanckehenri@gmail.com>
@nicor88 fixed merge conflicts and rebased again, let me know if there is anything else I can do to help |
@henriblancke nice job here, please address this #161 (comment) and then we can merge, if also @Jrmyy gives the ok. |
I will make a test this morning and let you know ! |
The test I made works therefore once my comment is resolved, this is a go for merging 👍🏻 |
@henriblancke please resolve this one #161 (comment) and we merge this and include in the next release. |
Signed-off-by: Henri Blancke <blanckehenri@gmail.com>
@Jrmyy @nicor88 thanks again for the review, I've addressed #161 (comment) |
Description
This change uploads seed files to s3 before creating the seed table. This makes larger seeds possible and removes the limitation of the athena query char limit.
It uploads the seeds as
json
to have better type casting support.OpenCSVSerde
is not good at casting timestamps and inferring correct data types. Since seeds are mostly smaller files this should be fine. Writing them as parquet adds too much complexity to this adapter.Checklist