Update Azure storage version

[pjsier]

Attempt to close #784. From what I can tell the updated version of azure-storage-blob doesn't have the same options for URL generation with is_emulated and custom_domain, so I've removed options that won't work anymore.

There's a lot in here, so any feedback is appreciated, thanks!

[pjsier]

It looks like the integration tests are the only checks failing, but now that the emulated mode is removed I’m not sure if those are fixable. Happy to address that however makes sense

[jschneier]

Is there a good way to assess how common the various Azure libraries are? Normally with these sorts of things the breaks are minimal and people just upgrade but this appears to be a larger one.

[pjsier]

I don't know of any ways to check usage numbers for specific library versions, and it doesn't seem like that's listed in the repo dependents page, but it is a pretty significant change. Would it make sense include this as a separate storage backend?

[larsrinn]

What exactly is missing to merge this? I'd like to relax the pinning of the azure-blob-storage package

[akx]

Would it make sense include this as a separate storage backend?

Making it azure2_storage (and the install extra similarly azure2) would probably make sense, so people who need to use the old azure-storage-blob package can keep using it.

Right now I'm looking into just vendoring this file (thanks for the work, btw!) since I'm using azure-storage-blob >= 12 for other things in my project.

[felixmeziere]

Need this please!!!!! Get_blob_to_stream bugging randomly for me....

[rododhendron]

Hello, guys, any update on this ?

[larsrinn]

I'd have some time to work on this in the next days. @jschneier what's your proposal on how to proceed here:

• Include it as a new storage backend?
• Replace the existing storage backend?
• Add a deprecation warning that this libraries dependency will change to azure-storage-blob>=12 in the next release and replace the existing backend in a few weeks?

---

Personally I'd be fine with simply replacing it. We're only using azure-storage-blob as a dependency of this package and I presume this is quite common. Hence, we wouldn't notice a change if this PR is merged. And given that this breaking change in azure-storage-blobs was released more than a year ago, I think it's fair to make the "new" version the default.

We're only using

[jschneier]

I think we should just replace it.

[pjsier]

I've got time to resolve the merge conflicts on this and make any changes if that works. I've been using this backend vendored for a while without any issues.

Is it alright to remove the integration tests that rely on the emulated mode? As far as I can tell that isn't supported anymore, but I haven't looked in a while

[jschneier]

You should remove anything that doesn't work. It'd be good to have a reasonable set of tests, of course.

[pjsier]

Sounds good! Yeah this would only apply to the integration tests, and it looks like the Azure backend is the only one that has integration tests. I updated the other tests, and on the last CI run those all passed

[pjsier]

I cleaned up the merge conflicts and dropped the integration tests since they only seemed to be used for Azure, but updated the other tests. Let me know if I should make any other changes, thanks!

[jschneier]

Are there any migration steps to document?

[pjsier]

It should work without any other changes. The main things to note are the removed custom settings, one of which is only related to tests

[jasoncabot]

Thanks @pjsier for the great work on this - I just tried it out with a tiny project and ran into an issue however.

The type of credential for ContainerClient has changed and now accepts TokenCredential (as well as the SAS / Access key)

I'm using a managed identity to avoid storing the access key in code or config, so my code looked like:

from azure.identity import DefaultAzureCredential
from azure_identity_credential_adapter import AzureIdentityCredentialAdapter

credential = DefaultAzureCredential()

AZURE_TOKEN_CREDENTIAL = AzureIdentityCredentialAdapter(credential, resource_id="https://storage.azure.com/.default")

Which wraps the new azure identity SDK and generates the correct token credential, however this PR failed with the following error(s depending on what I used as AZURE_TOKEN_CREDENTIAL):

TypeError: Unsupported credential: <azure_identity_credential_adapter.AzureIdentityCredentialAdapter object>
TypeError: Unsupported credential: AccessToken
azure.core.exceptions.ClientAuthenticationError: Operation returned an invalid status 'Server failed to authenticate the request. Make sure the value of Authorization header is formed correctly including the signature.'

I had to update my code to remove the intermediate wrapper and just pass in the TokenCredential directly from azure-identity now (which is much nicer!):

from azure.identity import DefaultAzureCredential

credential = DefaultAzureCredential()

AZURE_TOKEN_CREDENTIAL = credential

I'm just adding this here in case it helps anyone else who is using a similar set up

[pjsier]

@jasoncabot thanks for the info! For this PR I was trying to match functionality from the previous client, but it does seem like that's worth including here even if it's in a separate PR

[jasoncabot]

I don't believe there's a way of automatically mapping between MSIAuthentication and TokenCredentials so this is likely to be a breaking change for anyone authenticating on azure using only AZURE_TOKEN_CREDENTIAL

Is there somewhere (better than here?) that it could be documented?

[pjsier]

Ah I misunderstood, I see what you're saying now, and this would definitely be good to document. @jschneier would this make sense for the changelog? If so I can update that

[ppmdo]

Any update on this? Is there any help needed?

[lmmentel]

It would be great to get this merged! Any help needed?

[AlBlanc]

@pjsier thank you for your amazing work.

I am using your fork (branch update-azure-storage) and it looks like spaces are encoded twice in the url method.

It returns space in URLs as "%2520"

[pjsier]

@AlBlanc thanks for catching that! I haven't used this in a while, so if you have any insight on where in the method that can be fixed that would be great

[jschneier]

Looking at this for the next release.

It seems like there are two concerns

1. Migration or documentation instructions for the issue pointed out by @jasoncabot
2. Investigate the url bug from @AlBlanc

I'm inclined to focus on the first and to separately debug the second which is probably somewhere around the filepath_to_uricall combined with the other various .url properties

[jschneier]

I'm fairly certain just removing the filepath_to_uri call will fix the issue.

[pjsier]

Sounds great! I just cleaned up some conflicts and removed the filepath_to_uri call. Open to suggestions on the documentation

[jschneier]

@pjsier please remove the .travis.yml file and then I will pull the trigger.

[jschneier]

@pjsier can you also update the extra requires for azure in setup.cfg please.

[jschneier]

I also noticed there is still a reference to AZURE_EMULATED_MODE in the docs.

[jschneier]

Also custom_connection_string can go now.

[pjsier]

@jschneier just pushed those updates! I think the extras requires in setup.cfg to requiring azure-storage-blob>=12.0.0 should be sufficient though. Happy to change that if not