Skip to content

Feat/env name #11

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

Merged
merged 6 commits into from
Jun 11, 2024
Merged

Feat/env name #11

merged 6 commits into from
Jun 11, 2024

Conversation

HertugHelms
Copy link
Contributor

Hey guys

I came across this idea of adding env_name as a variable when instantiating an AzureStorageDriver. This enables us to do a thing like this:

sddatacube = azure_blob_interface.AzureStorageDriver(CONTAINER)
sdcoregistration = azure_blob_interface.AzureStorageDriver(
    COREGISTRATION_CONTAINER,
    env_name="AZURE_STORAGE_CONNECTION_STRING_COREGISTRATION"
)

I tested it out just now and it seems to work as intended and hopefully i have added the variable without adding it as a breaking change but feel free to comment here if you can see that it breaks a workflow that you have.

@HertugHelms HertugHelms requested review from radosuav and charalamm May 13, 2024 08:17
@radosuav
Copy link
Contributor

What is the difference/improvement from the way it's done currently? That we are not stuck with the hard-coded ACCOUNT_URL but can use different env variables for different connections?

@HertugHelms
Copy link
Contributor Author

Yes - so right now if you want to have multiple azurestoragedrivers in the same scope you would need to run

sd_one = azure_blob_interface.AzureStorageDriver(CONTAINER_ONE)
os.environ["ACCOUNT_URL"] = NEW_CONNECTION_STRING
sd_two = azure_blob_interface.AzureStorageDriver(CONTAINER_TWO)

and it's bad practice to manipulate environment variables mid-run i would say. This way we specify both container and ENV_NAME so we can run a container with certain environment variables and create azure storage drivers for each different connection string.

Another way to do it would be to just pass the connection string to the AzureStorageDriver along with container name and not have that library handle your environment variables, but i think that creates more backwards complications than this would.

@radosuav
Copy link
Contributor

Ok, sounds good, looks good :)

@charalamm
Copy link
Contributor

Looks good to me too! Nice, very useful

@HertugHelms HertugHelms merged commit fa8713c into main Jun 11, 2024
1 check passed
@HertugHelms HertugHelms deleted the feat/env-name branch June 11, 2024 06:30
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