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

Add support for access-token secrets #64

Merged
merged 7 commits into from
Jul 18, 2024

Conversation

mmaitre314
Copy link
Contributor

@samansmink
Copy link
Collaborator

Hey @mmaitre314 thanks for the PR, looks great!

To fix the CI failure, could you add the ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION=true env variable to the functional test CI jobs (like here https://github.com/duckdb/extension-ci-tools/blob/ebf18ed49b11e656adc20d721bf7dac2de15d439/.github/workflows/_extension_distribution.yml#L155)
Our Linux CI is a little broken at the moment and while we refactor a few things we need to have this workaround.

@mmaitre314
Copy link
Contributor Author

I added ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION in a couple of places but I am not very confident in where I added that... If I understand correctly, patch azure_http_state.patch is not getting applied and the code tries to include duckdb/common/http_state.hpp which does not exist anymore.

@samansmink
Copy link
Collaborator

Ah I see, sorry I missed that Azures CI is running against DuckDB's main branch. Normally we have CI target latest stable and let duckdb/duckdb CI take care of catching changes breaking compatiblity.

I've applied the patch to your branch, this should fix ci here. Then we can bump azure's version in duckdb/duckdb ci and remove the patch.

@mmaitre314
Copy link
Contributor Author

The formatting of HTTP stats changed a bit so I updated the Regex in the test to match.

Before: ss << "││ HTTP Stats: ││\n";

Now: ss << "││" + QueryProfiler::DrawPadded("Azure HTTP Stats", TOTAL_BOX_WIDTH - 4) + "││\n";

@samansmink samansmink merged commit d92b3b8 into duckdb:main Jul 18, 2024
18 checks passed
@samansmink
Copy link
Collaborator

thanks @mmaitre314!

@mmaitre314
Copy link
Contributor Author

Looking into updating docs as next step, should I wait until a new version of DuckDB is released?

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