duckdb / duckdb_azure

Azure extension for DuckDB
MIT License
50 stars 17 forks source link

Register in the client context a state to avoid reconnecting on Azure. #32

Closed quentingodeau closed 9 months ago

quentingodeau commented 9 months ago

This commit refactors the code (mostly moves it) to ease readability. It also add an Azure context that will be kept between the files access in a query. Previously if you were using a credential_chain provider when you query multiple files for each of them, we will have initiate a new connection and identify at the Azure AD (now Entra). Now this will be only performed once by query.

samansmink commented 9 months ago

hey @quentingodeau thanks a lot for the PR!

The CI failure for the distribution pipeline ci can be fixed by bumping the version of the reusable workflow duckdb/duckdb/.github/workflows/_extension_distribution.yml in .github/workflows/MainDistributionPipeline.yml to latest master of duckdb (currently: 64785543f882f8c9b2f81ae71fa43eadf1653573)

edit: and the Linux Release one seems some upstream server thats dead, i just ran into the same err on another pr

quentingodeau commented 9 months ago

Oh I misunderstood your comment, I rollback!

quentingodeau commented 9 months ago

Hi @samansmink, does the change on the workflow are the one you ask me to do ? I saw in your fork that you have update a sha, it that what I was suppose to do ?

quentingodeau commented 9 months ago

Hi again @samansmink, for info I have update the PR with the changes that you have done in the main branch :) regards, Quentin

samansmink commented 9 months ago

hi @quentingodeau sorry for the delay on all your PRs, i've been on holiday last week and very busy with the 0.10 release this week. I'll review this one and the rest asap!

quentingodeau commented 9 months ago

No problem! Take your time :) I known that I have move a lot of things and sorry for that, but I saw that the way the extension/unit test was build I was causing issue when you add some headers (because the includes options where not propagate with CMake)

quentingodeau commented 9 months ago

Greetings @samansmink thx for the review!!

samansmink commented 9 months ago

Looks great now, thanks again!