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

Feature/hierarchical namespace #49

Merged
merged 14 commits into from
Mar 13, 2024

Conversation

quentingodeau
Copy link
Contributor

Add support of DFS endpoint (#43)

@quentingodeau
Copy link
Contributor Author

I will wait for the PR#45 to update the tests

@quentingodeau quentingodeau force-pushed the feature/hierarchical-namespace branch from 5a9adfb to 3995ba1 Compare March 4, 2024 13:01
@quentingodeau quentingodeau force-pushed the feature/hierarchical-namespace branch from 3995ba1 to cdeb51c Compare March 4, 2024 18:02
@quentingodeau
Copy link
Contributor Author

Hi @samansmink,

I would like some help for setting up the hierarchical namespace tests, if that all right for you.
Also for the review, if needed, we can connect through discord to explain if things are not clear, or if you have question regarding some choice.

Regards,
Quentin

@quentingodeau
Copy link
Contributor Author

Note (to discuss):

  • management of **

Copy link
Collaborator

@samansmink samansmink left a comment

Choose a reason for hiding this comment

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

Hey @quentingodeau, amazing work, this looks great!

I've tried to set up the CI side of things but i'm running into some issues. Maybe we can have a chat? Its probably me misunderstanding the connection strings.

I've also added some comments.

Perhaps we can have a chat on discord? You can find me on the duckdb discord or just add directly: samansmink

src/azure_dfs_filesystem.cpp Show resolved Hide resolved
scripts/prepare_hierarchical_namespace_test.sh Outdated Show resolved Hide resolved
scripts/prepare_hierarchical_namespace_test.sh Outdated Show resolved Hide resolved
test/sql/cloud/hierarchical_namespace.test Outdated Show resolved Hide resolved
@quentingodeau quentingodeau marked this pull request as ready for review March 6, 2024 07:12
Make tests work with DFS and BLOB to use the same storage account (must be created with the
hierarchical namespace option enabled to run abfss tests)
@quentingodeau
Copy link
Contributor Author

Hi @samansmink, sorry for the delay!
I have update the tests and try to add sens to them. Take time to review a bit what I have done there because I try to adapt so we can use the same storage account with blob and dfs tests. but I had to add contains(file, '.') for the test_data_integrity.testto make them work...
If something failed do not bother I will take a look tomorrow :)
Regards,
Quentin

@quentingodeau
Copy link
Contributor Author

Hum the cloud testing didn't run. Are they trigger manually?

Copy link
Collaborator

@samansmink samansmink left a comment

Choose a reason for hiding this comment

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

Thanks for all the changes, this looks great! Added some small comments

scripts/upload_test_files_to_azure.sh Outdated Show resolved Hide resolved
data/README.md Outdated Show resolved Hide resolved
@samansmink
Copy link
Collaborator

@quentingodeau they don't run on PRs for security purposes. They will run on main, and I will check them manually as part of review.

@samansmink
Copy link
Collaborator

@samansmink samansmink merged commit 86f39d7 into duckdb:main Mar 13, 2024
18 checks passed
@quentingodeau quentingodeau deleted the feature/hierarchical-namespace branch March 17, 2024 13:26
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.

2 participants