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

Improve CI #45

Merged
merged 11 commits into from
Mar 4, 2024
Merged

Improve CI #45

merged 11 commits into from
Mar 4, 2024

Conversation

samansmink
Copy link
Collaborator

This PR:

  • cleans up CI a bit by merging the functional tests together
  • adds a new workflow for tests that use an actual azure account to test against

CI is running on my fork here: https://github.com/samansmink/duckdb_azure/actions/runs/8096308985 where i also added the test credentials.

The cloud functional tests allow testing against real azure infrastructure instead of the azurite test server. Things that are now tested include:

  • Creating and using a secret using the credential_chain provider with the env chain
  • Testing the az cli login
  • Testing the new SPN secret provider
  • Confirming that the http stats of azure queries make sense

@samansmink
Copy link
Collaborator Author

Hey @quentingodeau! I added some real-world tests with the SERVICE_PRINCIPAL provider. They won't run on PR's for obvious security reasons, but at least now CI can test master branch with Azure a little bit.

If you proceed with #44 or #43, I can set up stuff in our azure test account to have things properly tested in CI here.

@carlopi
Copy link
Contributor

carlopi commented Mar 4, 2024

LGTM!

Things that I though could be nice adding:

  • a test showing what's the minimal needed to read from a public bucket (without setting secrets, or a minimal amount of them)
  • a test showing that bucket has not been tampered (question is a bit fuzzy, but basically something saying stuff is still in a valid state after our read / unsure if there is some metric like time of last modification to the bucket available from azure )

@samansmink
Copy link
Collaborator Author

Thanks for the review @carlopi and @chrisiou. I've added:

  • a test for the unauthenticated requests
  • a test that checks that all files in the buckets are as expected, these are run for both azurite and azure to ensure we don't accidentally mess up the bucket.

@samansmink samansmink merged commit 9e3e5b8 into duckdb:main Mar 4, 2024
18 checks passed
@samansmink samansmink deleted the improve-ci branch March 4, 2024 16:06
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.

3 participants