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.

Sign up for GitHub

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

Jump to bottom

Add support of providing CA path directory for Azure SDK's libcurl backend #867

Merged
merged 1 commit into from
May 29, 2024
Merged

Add support of providing CA path directory for Azure SDK's libcurl backend #867

merged 1 commit into from
May 29, 2024

Conversation

phoebusm
Copy link
Collaborator

@phoebusm phoebusm commented Sep 18, 2023 •
edited
Loading

Reference Issues/PRs

#514

What does this implement/fix? How does it work (high level)? Highlight notable design decisions.

Add support of providing CA path directory for Azure SDK's underlying libcurl. This PR includes two parts:

  1. Upgrade azure-cpp-sdk
  2. Corresponding changes and test for ArcticDB to allow user to update the above option

This PR will be depended by the feature of auto supplying Linux OS-default CA cert to Azure's libcurl backend.

Any other comments?

Checklist

Checklist for code changes...
  • Have you updated the relevant docstrings and documentation?
  • Is this contribution tested against all ArcticDB's features?
  • Do all exceptions introduced raise appropriate error messages?
  • Are API changes highlighted in the PR description?
  • Is the PR labelled as enhancement or bug so it appears in autogenerated release notes?

@phoebusm phoebusm added the enhancement New feature or request label Sep 18, 2023
This was referenced Sep 18, 2023
Closed
Closed
@phoebusm
Copy link
Collaborator Author

Pending corresponding patch for conda environment. Conda does not support patching sources locally

@phoebusm phoebusm changed the title Add support of providing CA path directory for Azure SDK's libcurl backend WIP: Add support of providing CA path directory for Azure SDK's libcurl backend Sep 18, 2023
@phoebusm phoebusm mentioned this pull request Sep 19, 2023
Merged
5 tasks
poodlewars
poodlewars previously approved these changes Sep 19, 2023
View reviewed changes
Copy link
Collaborator

@poodlewars poodlewars left a comment

Choose a reason for hiding this comment

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

Great! Thank you.

Have you tested that this works to connect to a real Azure blob store (not just an Azurite emulated one)?

python/arcticdb/adapters/azure_library_adapter.py Show resolved Hide resolved
python/arcticdb/adapters/azure_library_adapter.py Outdated Show resolved Hide resolved
python/arcticdb/adapters/azure_library_adapter.py Outdated Show resolved Hide resolved
python/arcticdb/arctic.py Outdated Show resolved Hide resolved
python/arcticdb/arctic.py Outdated Show resolved Hide resolved
@phoebusm phoebusm force-pushed the feature/azure_libcurl_auto_capath_support branch 5 times, most recently from 9c3c291 to 1eb8167 Compare September 25, 2023 13:20
@phoebusm
Copy link
Collaborator Author

Azure/azure-sdk-for-cpp#4982
Awaiting above PR so that the pin of Azure C++ SDK on Conda can be updated

@phoebusm
Copy link
Collaborator Author

Azure/azure-sdk-for-cpp#4982 Awaiting above PR so that the pin of Azure C++ SDK on Conda can be updated

PR merged. Awaiting Azure SDK pin being updated on conda feedstock

@phoebusm
Copy link
Collaborator Author

Azure/azure-sdk-for-cpp#4982 (comment)

...this change should be available on vcpkg at some time next week with the October release of the Azure SDK.

@phoebusm
Copy link
Collaborator Author

phoebusm commented Oct 3, 2023

This PR will be put on-hold as Azure/azure-sdk-for-cpp#4982 (comment) stated that the PR on Azure SDK won't be officially release until 10th Nov.

@poodlewars poodlewars marked this pull request as draft October 25, 2023 17:43
@phoebusm phoebusm mentioned this pull request Jan 8, 2024
Closed
@phoebusm phoebusm force-pushed the feature/azure_libcurl_auto_capath_support branch from 1eb8167 to 5c4924d Compare April 12, 2024 17:23
@phoebusm phoebusm linked an issue Apr 25, 2024 that may be closed by this pull request
Azure storage linux support for https on non-RHEL distribution #514
Closed
@phoebusm phoebusm force-pushed the feature/azure_libcurl_auto_capath_support branch 2 times, most recently from 657d396 to 25fe885 Compare April 30, 2024 16:02
@phoebusm phoebusm changed the title WIP: Add support of providing CA path directory for Azure SDK's libcurl backend Add support of providing CA path directory for Azure SDK's libcurl backend Apr 30, 2024
@phoebusm phoebusm marked this pull request as ready for review April 30, 2024 16:06
@phoebusm phoebusm force-pushed the feature/azure_libcurl_auto_capath_support branch 2 times, most recently from 6f79f58 to e85b4d3 Compare April 30, 2024 17:25
@poodlewars poodlewars self-requested a review May 14, 2024 11:42
poodlewars
poodlewars requested changes May 14, 2024
View reviewed changes
python/tests/integration/arcticdb/test_arctic.py Outdated Show resolved Hide resolved
python/tests/integration/arcticdb/test_arctic.py Show resolved Hide resolved
python/arcticdb/adapters/azure_library_adapter.py Outdated Show resolved Hide resolved
python/arcticdb/storage_fixtures/azure.py Outdated Show resolved Hide resolved
python/arcticdb/storage_fixtures/azure.py Outdated Show resolved Hide resolved
cpp/arcticdb/storage/azure/azure_storage.cpp Outdated Show resolved Hide resolved
cpp/vcpkg.json Show resolved Hide resolved
docs/mkdocs/docs/api/arctic_uri.md Outdated Show resolved Hide resolved
poodlewars
poodlewars requested changes May 17, 2024
View reviewed changes
docs/mkdocs/docs/api/arctic_uri.md Outdated Show resolved Hide resolved
docs/mkdocs/docs/api/arctic_uri.md Outdated Show resolved Hide resolved
docs/mkdocs/docs/api/arctic_uri.md Outdated Show resolved Hide resolved
python/arcticdb/adapters/s3_library_adapter.py Outdated Show resolved Hide resolved
python/tests/integration/arcticdb/test_arctic.py Show resolved Hide resolved
python/tests/integration/arcticdb/test_arctic.py Outdated Show resolved Hide resolved
python/tests/conftest.py Show resolved Hide resolved
@poodlewars @phoebusm
-Set ca path and directory automatically
7ae8e2e 
Revoke removing assert

Remove useless ca cert path in non ssl enabled testing environment

Address PR comment

Better test

Address PR comments

Update docs/mkdocs/docs/api/arctic_uri.md

Co-authored-by: Alex Seaton <[email protected]>
@phoebusm phoebusm force-pushed the feature/azure_libcurl_auto_capath_support branch from 8fd0db9 to 7ae8e2e Compare May 28, 2024 16:29
@phoebusm phoebusm merged commit 5dd6a64 into master May 29, 2024
114 checks passed
@phoebusm phoebusm deleted the feature/azure_libcurl_auto_capath_support branch May 29, 2024 10:23
@jjerphan jjerphan mentioned this pull request Jun 24, 2024
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@poodlewars poodlewars poodlewars approved these changes

@alexowens90 alexowens90 Awaiting requested review from alexowens90 alexowens90 is a code owner

@willdealtry willdealtry Awaiting requested review from willdealtry willdealtry is a code owner

Assignees

@phoebusm phoebusm

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Azure storage linux support for https on non-RHEL distribution
2 participants
@phoebusm @poodlewars