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

chore: single storage client support multiple blob-containers #740

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Meir017
Copy link
Contributor

@Meir017 Meir017 commented Oct 20, 2024

closes #739

@TalZaccai TalZaccai requested a review from badrishc October 22, 2024 20:38
@badrishc
Copy link
Contributor

I don't see how this solution would work. The reason we create a different factory each time is that we need to initialize the factory with a different baseName for each factory. That way, the instances created by the factory will all use the same base name. See GarnetServerOptions->GetInitializedDeviceFactory for how this is used.

@Meir017
Copy link
Contributor Author

Meir017 commented Oct 23, 2024

I don't see how this solution would work. The reason we create a different factory each time is that we need to initialize the factory with a different baseName for each factory. That way, the instances created by the factory will all use the same base name. See GarnetServerOptions->GetInitializedDeviceFactory for how this is used.

I see, I was confused here the scope of AzureStorageNamedDeviceFactory, I think we need another layer for the AzureStorageNamedDeviceFactory as I think for a single blob storage we should create at most a single client, but a single client could interact with multiple blob containers - and currently the code supports a 1:1 relationship between storage-account and blob-container

@Meir017 Meir017 marked this pull request as draft October 23, 2024 06:13
@Meir017 Meir017 changed the title chore: make garnet server options DeviceFactoryCreator creation be lazy chore: single storage client support multiple blob-containers Oct 23, 2024
@badrishc
Copy link
Contributor

Agreed, something is off re: the abstraction right now. We need to be able to define the base name independently of the storage library instance.

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.

Initialize azure blob storage client once during startup
2 participants