upgrade azure-storage and pass token_credential argument (#3005) #3008
Conversation
…rvice and fix spotify#3005 azure-storage==0.36 didn't support `token_credential` argument in `BlockBlobService` init. Upgrade to the new package `azure-storage-blob` and use the last version that still supports `BlockBlobService` (it's deprecated, but that can be upgraded another time) Fixes spotify#3005 by passing `token_credential` argument. I also had to explicitely default the protocol to "https", otherwise the initialization fails when we provide a token_credential argument.
vbarbaresi
requested review from
dlstadther,
honnix and
Tarrasch
as code owners
|
gentle bump, could it be merged? |
| @@ -74,7 +75,8 @@ def connection(self): | |||
| connection_string=self.kwargs.get("connection_string"), | |||
| endpoint_suffix=self.kwargs.get("endpoint_suffix"), | |||
| custom_domain=self.kwargs.get("custom_domain"), | |||
| is_emulated=self.kwargs.get("is_emulated") or False) | |||
| is_emulated=self.kwargs.get("is_emulated", False), | |||
| token_credential=self.kwargs.get("token_credential")) | |||
There was a problem hiding this comment.
will azure-storage<=0.36 allow for the passing of token_credential without fail? (even if without impact)
There was a problem hiding this comment.
no, it immediately fails
>>> blockblobservice.BlockBlobService(account_name="a", token_credential="a")
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: __init__() got an unexpected keyword argument 'token_credential'
do you prefer to keep some compatibility in here in case users aren't able to upgrade their packages?
There was a problem hiding this comment.
Oh wait, i see the hard update on az dependencies anyway.
So either we make this change, as is, and include it as an explicit breaking change. Or we explore options for enabling both.
I'm not immediately thinking of an easy way to keep compatibility.
azure-storage==0.36 is also from 2017...
There was a problem hiding this comment.
I can refactor these keyword arguments in a dict, and add token_credential argument only if we're using the new azure package:
from pkg_resources import parse_version
new_azure_package = parse_version(azure.storage.blob.__version__) >= parse_version("2.1.0")(Under a try/except AttributeError because this attribute didn't exist in the old package, it's azure.storage.__version__ )
There was a problem hiding this comment.
I pushed it in another branch 0566938
I can cherry-pick this here, it's up to you
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If closed, you may revisit when your time allows and reopen! Thank you for your contributions. |
Motivation and context
azure-storage==0.36 didn't support
token_credentialargument inBlockBlobServiceinit.I upgraded to the new package
azure-storage-bloband use the last version that still supportsBlockBlobService(it's deprecated, but that can be upgraded another time)Description
Fixes #3005 by passing
token_credentialargument.I also had to explicitely default the protocol to "https", otherwise the initialization fails when we provide a token_credential argument.
I successfully ran azureblob unit tests