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

Azure transfer driver #1019

Merged
merged 6 commits into from
Apr 18, 2024
Merged

Azure transfer driver #1019

merged 6 commits into from
Apr 18, 2024

Conversation

gcglinton
Copy link
Contributor

Draft

This implements an Azure transfer driver, to complement the S3 one from #1010.

Many of the test are being skipped, because there's no good/easy way to mock/fake the Azure platform.
Fix incorrect variables and methods that came from the S3 tests
Forgot to include 'azure-storage-blob'..
- Remove Matrix, only run on Ubuntu 22.04
- Publish coverage and test results
- Bump checkout and upload-artifact to latest
Copy link

Test Results

209 tests   201 ✅  14s ⏱️
  1 suites    8 💤
  1 files      0 ❌

Results for commit 3b294ce.

@petersilva petersilva marked this pull request as ready for review April 18, 2024 00:33
@petersilva petersilva merged commit 5081105 into development Apr 18, 2024
@gcglinton
Copy link
Contributor Author

Huh.. I didn't mean to merge this so quickly, but... great!

@gcglinton gcglinton deleted the Issue1015_AzureTransferDriver branch April 18, 2024 12:21
@petersilva
Copy link
Contributor

commit early-commit often... the gethttps thing... will it provide the proper authentication?

@gcglinton
Copy link
Contributor Author

gcglinton commented Apr 18, 2024

commit early-commit often...

❤️

the gethttps thing... will it provide the proper authentication?

No. It's just giving the URL to the file in question. The assumption would be that the publisher would have anonymous access turned on for the account/container in question, so SAS tokens wouldn't be required. If auth was needed, the consumer could just use the Azure transfer driver, which does accept credentials.

Happy to change that if you want, but the creds returned will be those of the publisher, so... that might not be ideal. Unless we add a config option for "publish" creds that the driver uses when it returns the HTTPs URL.

@petersilva
Copy link
Contributor

fair point... forget it for now.

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