search

duckdb / dbt-duckdb

dbt (http://getdbt.com) adapter for DuckDB (http://duckdb.org)
Apache License 2.0
852 stars 77 forks source link

Create release on current master branch #150

Closed JasP19 closed 1 year ago

JasP19 commented 1 year ago

Please could you create a release for the current changes in the master branch?

I experienced some unexpected behaviour relating to the use of AWS credentials - the README mentions how to use the use_credential_provider flag for AWS but in the latest release (1.4.1), this is not currently supported (due to the lack of the credentials.py file in the adapters directory).

Installing the package directly from git results in the use_credential_provider flag working as expected. Unfortunately, our build pipeline only allows installation of packages from PyPI. As such, we are requesting a new release with the current changes on master please.

jwills commented 1 year ago

You got it, I’ll cut one in a couple of hours.

jwills commented 1 year ago

Hey @JasP19 and @arachnegl: looked at this and I don't totally understand-- the credentials.py file is just a refactoring, the class itself still exists in 1.4.1, it's just in the connections.py file, and the use_credential_provider should be supported there. Is it not working?

jwills commented 1 year ago

Ah, I see the bug fix you want-- okay, cutting a release ~ now

JasP19 commented 1 year ago

Thanks for the quick reply! In 1.4.1, when I attempt to fetch a parquet file from S3 with use_credential_provider: aws, I am getting a 400 error. When I set the s3_access_key_id and s3_secret_access_key configuration, it works as expected.

After some investigation, we believed it was due to the lack of the credentials.py file. We switched to the version on master and things started working as expected. Having had a quick look at connections.py, I believe the key difference compared to master is this line: https://github.com/jwills/dbt-duckdb/blob/master/dbt/adapters/duckdb/credentials.py#L182

Essentially, the standard boto3 session does not set a region by default which means that s3_region gets overwritten by an empty value. This is a common cause for 400 errors when accessing files on S3. Thanks for your work!

jwills commented 1 year ago

yah, it's this fix you want i think: https://github.com/jwills/dbt-duckdb/pull/138

...which is in master, but not 1.4.1.

jwills commented 1 year ago

Okay, 1.4.2 is up on pypi: https://pypi.org/project/dbt-duckdb/

gonna post a link in the release section in two shakes

JasP19 commented 1 year ago

Awesome stuff. Thanks for the quick work to get this done! I've given 1.4.2 a test and things are working as expected.