-
Notifications
You must be signed in to change notification settings - Fork 12
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
Support reading s3 config from config file #36
Conversation
…a parquet dest receiver to write parquet file from the table or query.
…tocol to populate tables from parquet file.
- adds a GUC, named "pg_parquet.enable_copy_hooks", which enables/disabled the hook, - exposes parquet dest receiver api so that the dest receiver could be used independently from the hook.
- adds "parquet.metadata", "parquet.file_metadata", and "parquet.kv_metadata" udfs to inspect the metadata of a parquet file.
echo AWS_SECRET_ACCESS_KEY=${{ env.AWS_SECRET_ACCESS_KEY }} >> /tmp/.env | ||
echo AWS_REGION=${{ env.AWS_REGION }} >> /tmp/.env | ||
echo AWS_S3_TEST_BUCKET=${{ env.AWS_S3_TEST_BUCKET }} >> /tmp/.env | ||
echo PG_PARQUET_TEST=${{ env.PG_PARQUET_TEST }} >> /tmp/.env |
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.
would be kind of nice to write an actual .aws/credentials to confirm this PR works
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.
we unset env vars in the s3 tests while reading from config.
|
||
pub async fn get_s3_object_store(bucket_name: &str) -> AmazonS3 { | ||
// try loading the .env file | ||
dotenvy::from_path("/tmp/.env").ok(); |
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.
weird absolute path, was this meant for a test?
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.
yeah, but we can use .env in current path.
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.
current path causes problems as it looks up the folder where the binary exists. I think /tmp
is ok. It is already fail safe when we call .ok()
which means swallow error if any.
7446ed6
to
e44d6f5
Compare
Moved to #26 |
No description provided.