-
Notifications
You must be signed in to change notification settings - Fork 110
Introduce connect client tests #507
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
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,8 @@ | ||
URL = "http://127.0.0.1:10000" | ||
HOST = "127.0.0.1:10000" | ||
URL = f"http://{HOST}" | ||
ACCOUNT_NAME = "devstoreaccount1" | ||
KEY = "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==" # NOQA | ||
CONN_STR = f"DefaultEndpointsProtocol=http;AccountName={ACCOUNT_NAME};AccountKey={KEY};BlobEndpoint={URL}/{ACCOUNT_NAME};" # NOQA | ||
SAS_TOKEN = "not-a-real-sas-token" | ||
DEFAULT_VERSION_ID = "1970-01-01T00:00:00.0000000Z" | ||
LATEST_VERSION_ID = "2022-01-01T00:00:00.0000000Z" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,205 @@ | ||
from unittest import mock | ||
|
||
import azure.storage.blob | ||
import pytest | ||
from azure.storage.blob.aio import BlobServiceClient as AIOBlobServiceClient | ||
|
||
from adlfs import AzureBlobFile, AzureBlobFileSystem | ||
from adlfs.tests.constants import ACCOUNT_NAME, CONN_STR, HOST, KEY, SAS_TOKEN | ||
from adlfs.utils import __version__ as __version__ | ||
|
||
|
||
@pytest.fixture() | ||
def mock_from_connection_string(mocker): | ||
return mocker.patch.object( | ||
AIOBlobServiceClient, | ||
"from_connection_string", | ||
autospec=True, | ||
side_effect=AIOBlobServiceClient.from_connection_string, | ||
) | ||
|
||
|
||
@pytest.fixture() | ||
def mock_service_client_init(mocker): | ||
return mocker.patch.object( | ||
AIOBlobServiceClient, | ||
"__init__", | ||
autospec=True, | ||
side_effect=AIOBlobServiceClient.__init__, | ||
) | ||
|
||
|
||
def get_expected_client_init_call( | ||
account_url, | ||
credential=None, | ||
location_mode="primary", | ||
user_agent=f"adlfs/{__version__}", | ||
): | ||
call_kwargs = { | ||
"account_url": account_url, | ||
} | ||
if credential is not None: | ||
call_kwargs["credential"] = credential | ||
if location_mode is not None: | ||
call_kwargs["_location_mode"] = location_mode | ||
if user_agent is not None: | ||
call_kwargs["user_agent"] = user_agent | ||
return mock.call(mock.ANY, **call_kwargs) | ||
|
||
|
||
def get_expected_client_from_connection_string_call( | ||
conn_str=None, | ||
user_agent=f"adlfs/{__version__}", | ||
): | ||
call_kwargs = { | ||
"conn_str": conn_str, | ||
} | ||
if user_agent is not None: | ||
call_kwargs["user_agent"] = user_agent | ||
return mock.call(**call_kwargs) | ||
|
||
|
||
def assert_client_create_calls( | ||
mock_client_create_method, | ||
expected_create_call, | ||
expected_call_count=1, | ||
): | ||
expected_call_args_list = [expected_create_call for _ in range(expected_call_count)] | ||
assert mock_client_create_method.call_args_list == expected_call_args_list | ||
|
||
|
||
def ensure_no_api_calls_on_close(file_obj): | ||
# Marks the file-like object as closed to prevent any API calls during an invocation of | ||
# close(), which can occur during garbage collection or direct invocation. | ||
# | ||
# This is important for test cases where we do not want to make an API request whether: | ||
# | ||
# * The test would hang because Azurite is not configured to use SSL and adlfs always sets SSL | ||
# for SDK clients created via their initializer. | ||
# | ||
# * The filesystem is configured to use the secondary location which can by-pass the location | ||
# that Azurite is running. | ||
file_obj.closed = True | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"fs_kwargs,expected_client_init_call", | ||
[ | ||
( | ||
{"account_name": ACCOUNT_NAME, "account_key": KEY}, | ||
get_expected_client_init_call( | ||
account_url=f"https://{ACCOUNT_NAME}.blob.core.windows.net", | ||
credential=KEY, | ||
), | ||
), | ||
( | ||
{"account_name": ACCOUNT_NAME, "credential": SAS_TOKEN}, | ||
get_expected_client_init_call( | ||
account_url=f"https://{ACCOUNT_NAME}.blob.core.windows.net", | ||
credential=SAS_TOKEN, | ||
), | ||
), | ||
( | ||
{"account_name": ACCOUNT_NAME, "sas_token": SAS_TOKEN}, | ||
get_expected_client_init_call( | ||
account_url=f"https://{ACCOUNT_NAME}.blob.core.windows.net?{SAS_TOKEN}", | ||
), | ||
), | ||
# Anonymous connection | ||
( | ||
{"account_name": ACCOUNT_NAME}, | ||
get_expected_client_init_call( | ||
account_url=f"https://{ACCOUNT_NAME}.blob.core.windows.net", | ||
location_mode=None, | ||
), | ||
), | ||
# Override host | ||
( | ||
{ | ||
"account_name": ACCOUNT_NAME, | ||
"account_host": HOST, | ||
"sas_token": SAS_TOKEN, | ||
}, | ||
get_expected_client_init_call( | ||
account_url=f"https://{HOST}?{SAS_TOKEN}", | ||
), | ||
), | ||
# Override location mode | ||
( | ||
{ | ||
"account_name": ACCOUNT_NAME, | ||
"account_key": KEY, | ||
"location_mode": "secondary", | ||
}, | ||
get_expected_client_init_call( | ||
account_url=f"https://{ACCOUNT_NAME}.blob.core.windows.net", | ||
credential=KEY, | ||
location_mode="secondary", | ||
), | ||
), | ||
( | ||
{ | ||
"account_name": ACCOUNT_NAME, | ||
"credential": SAS_TOKEN, | ||
"location_mode": "secondary", | ||
}, | ||
get_expected_client_init_call( | ||
account_url=f"https://{ACCOUNT_NAME}.blob.core.windows.net", | ||
credential=SAS_TOKEN, | ||
location_mode="secondary", | ||
), | ||
), | ||
( | ||
{ | ||
"account_name": ACCOUNT_NAME, | ||
"sas_token": SAS_TOKEN, | ||
"location_mode": "secondary", | ||
}, | ||
get_expected_client_init_call( | ||
account_url=f"https://{ACCOUNT_NAME}.blob.core.windows.net?{SAS_TOKEN}", | ||
location_mode="secondary", | ||
), | ||
), | ||
], | ||
) | ||
def test_connect_initializer( | ||
storage: azure.storage.blob.BlobServiceClient, | ||
mock_service_client_init, | ||
fs_kwargs, | ||
expected_client_init_call, | ||
): | ||
fs = AzureBlobFileSystem(skip_instance_cache=True, **fs_kwargs) | ||
assert_client_create_calls(mock_service_client_init, expected_client_init_call) | ||
|
||
f = AzureBlobFile(fs, "data/root/a/file.txt", mode="wb") | ||
f.connect_client() | ||
ensure_no_api_calls_on_close(f) | ||
assert_client_create_calls( | ||
mock_service_client_init, | ||
expected_client_init_call, | ||
expected_call_count=2, | ||
) | ||
|
||
|
||
def test_connect_connection_str( | ||
storage: azure.storage.blob.BlobServiceClient, mock_from_connection_string | ||
): | ||
fs = AzureBlobFileSystem( | ||
account_name=storage.account_name, | ||
connection_string=CONN_STR, | ||
skip_instance_cache=True, | ||
) | ||
expected_from_connection_str_call = get_expected_client_from_connection_string_call( | ||
conn_str=CONN_STR, | ||
) | ||
assert_client_create_calls( | ||
mock_from_connection_string, expected_from_connection_str_call | ||
) | ||
|
||
f = AzureBlobFile(fs, "data/root/a/file.txt", mode="rb") | ||
f.connect_client() | ||
assert_client_create_calls( | ||
mock_from_connection_string, | ||
expected_from_connection_str_call, | ||
expected_call_count=2, | ||
) |
This file was deleted.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the user agent is never none, does there need to be an if statement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good callout. I mainly did it out of symmetry with the other kwargs and we could leverage that if we ever add the ability to update the user agent in adlfs. But we don't today so we can always set it in the expected call.