-
Notifications
You must be signed in to change notification settings - Fork 117
Respect protocol tuple in removal from full URIs
#512
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1776,15 +1776,15 @@ def test_find_with_prefix(storage): | |
| ] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("proto", [None, "abfs://", "az://"]) | ||
| @pytest.mark.parametrize("proto", [None, "abfs://", "az://", "abfss://"]) | ||
| @pytest.mark.parametrize("path", ["container/file", "container/file?versionid=1234"]) | ||
| def test_strip_protocol(proto, path): | ||
| assert ( | ||
| AzureBlobFileSystem._strip_protocol(f"{proto}{path}" if proto else path) == path | ||
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("proto", ["", "abfs://", "az://"]) | ||
| @pytest.mark.parametrize("proto", ["", "abfs://", "az://", "abfss://"]) | ||
| @pytest.mark.parametrize("key", ["file", "dir/file"]) | ||
| @pytest.mark.parametrize("version_aware", [True, False]) | ||
| @pytest.mark.parametrize("version_id", [None, "1970-01-01T00:00:00.0000000Z"]) | ||
|
|
@@ -1805,6 +1805,41 @@ def test_split_path(storage, proto, key, version_aware, version_id): | |
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("unsupported_proto", ["unsupported", "wasb", "wasbs"]) | ||
| def test_can_update_default_supported_protocols(storage, mocker, unsupported_proto): | ||
| mocker.patch.object( | ||
| AzureBlobFileSystem, | ||
| "protocol", | ||
| AzureBlobFileSystem.protocol + (unsupported_proto,), | ||
| ) | ||
|
Comment on lines
+1810
to
+1814
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: if protocol here is meant to be overridden, lets not use mock here since thats specifically for testing. Would be great to show how to override the protocol
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'd prefer if we stick with using Technically, we can get around this by adding a fixture that restores the original Other option, I'm fine with is defining a subclass of Thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make sense! just a nit. i think just having some docs around overriding
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can set the .protocol on the instance instead of the class. That's the only real use case for "overriding" that is likely to happen.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @martindurant I actually went with the approach of just overriding the So unfortunately, being able to override the I did explore what it would take to be able to override Thoughts?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK yes, you are right I think. |
||
| fs = AzureBlobFileSystem( | ||
| account_name=storage.account_name, | ||
| connection_string=CONN_STR, | ||
| skip_instance_cache=True, | ||
| ) | ||
| assert unsupported_proto in fs.protocol | ||
| assert ( | ||
| fs._strip_protocol(f"{unsupported_proto}://container/file") == "container/file" | ||
| ) | ||
| assert fs.split_path(f"{unsupported_proto}://container/file") == ( | ||
| "container", | ||
| "file", | ||
| None, | ||
| ) | ||
|
|
||
|
|
||
| def test_can_restrict_protocol_to_single_string(storage, mocker): | ||
| mocker.patch.object(AzureBlobFileSystem, "protocol", "abfs") | ||
| fs = AzureBlobFileSystem( | ||
| account_name=storage.account_name, | ||
| connection_string=CONN_STR, | ||
| skip_instance_cache=True, | ||
| ) | ||
| assert fs.protocol == "abfs" | ||
| assert fs._strip_protocol("abfs://container/file") == "container/file" | ||
| assert fs.split_path("abfs://container/file") == ("container", "file", None) | ||
|
|
||
|
|
||
| async def test_details_versioned(storage): | ||
| from azure.storage.blob import BlobProperties | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.