-
Notifications
You must be signed in to change notification settings - Fork 173
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
duckdb filesystem custom secrets #2017
base: devel
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for dlt-hub-docs canceled.
|
).fetchone()[0] | ||
|
||
# TODO: what is with windows? | ||
is_default_secrets_directory = current_secret_directory.endswith("/.duckdb/stored_secrets") |
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.
not going to work because windows use backslashes. ie.
D SELECT current_setting('secret_directory') AS secret_directory;
┌──────────────────────────────────────────┐
│ secret_directory │
│ varchar │
├──────────────────────────────────────────┤
│ C:\Users\rudolfix\.duckdb\stored_secrets │
└──────────────────────────────────────────┘
please use Path to compare paths
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.
done
destinations_configs(all_buckets_filesystem_configs=True, bucket_subset=(AWS_BUCKET,)), | ||
ids=lambda x: x.name, | ||
) | ||
def test_secrets_management(destination_config: DestinationTestConfiguration) -> None: |
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.
do we need this test in the load? if you move it to tests/pipelines
it will execute on windows as well. then you'll see if you normalize the paths correctly
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.
moved it
8f5d457
to
a0828a9
Compare
test secrets on all platforms
a0828a9
to
cd30eb8
Compare
pytest.ini
Outdated
@@ -1,7 +1,7 @@ | |||
[pytest] | |||
pythonpath= dlt docs/website/docs | |||
norecursedirs= .direnv .eggs build dist | |||
addopts= -v --showlocals --durations 10 |
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.
ok, somehow it does not work on ci...
5741abb
to
40b8b80
Compare
@@ -89,6 +89,7 @@ alembic = {version = ">1.10.0", optional = true} | |||
paramiko = {version = ">=3.3.0", optional = true} | |||
sqlglot = {version = ">=20.0.0", optional = true} | |||
db-dtypes = { version = ">=1.2.0", optional = true } | |||
aiohttp = { version = ">=3.9", optional = true } |
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.
this seems to be needed for python 3.12
Description
Note:
I tried implementing changing the secrets location in create_authentication, but somehow I could not get it right. I think this solution with warning the user should be good for now.