-
Notifications
You must be signed in to change notification settings - Fork 2
#245-adding-backend-inference-for-the-path-api #246
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
base: main
Are you sure you want to change the base?
Changes from all commits
c44fb91
f879064
61b1248
e540f24
18adad6
368507c
6686955
73e155f
8414ce8
98ba24f
ebef6c0
037c904
0dc7f41
f33bcaf
58a6a6e
5ae1a00
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 |
---|---|---|
@@ -1,4 +1,5 @@ | ||
# Unreleased | ||
|
||
## Refactorings | ||
* #186: Integration test for correctness of UDF path generation, using as_udf_path and pathlike | ||
* #186: Integration test for correctness of UDF path generation, using as_udf_path and pathlike | ||
* #245: Add backend inference for the Path-API |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,14 @@ | ||
from __future__ import annotations | ||
|
||
import io | ||
import re | ||
import tarfile | ||
from collections.abc import ByteString | ||
|
||
import pytest | ||
|
||
import exasol.bucketfs as bfs | ||
from exasol.bucketfs._path import infer_path | ||
|
||
|
||
@pytest.fixture | ||
|
@@ -101,3 +103,120 @@ def test_write_delete(backend_aware_bucketfs_params, children_poem, classic_poem | |
poem_path1.rm() | ||
expected_names = {"classic", "highlands.txt"} | ||
assert _collect_all_names(poems_root) == expected_names | ||
|
||
|
||
@pytest.fixture | ||
def require_saas_params(backend_aware_onprem_bucketfs_params, use_onprem): | ||
if not use_onprem: | ||
pytest.skip("Skipped as on-premise backend is not selected") | ||
return backend_aware_onprem_bucketfs_params | ||
|
||
|
||
def test_infer_path_onprem(require_onprem_bucketfs_params): | ||
""" | ||
Creates the PathLike and validates it. | ||
""" | ||
if backend == "saas": | ||
pytest.skip() | ||
host_port = re.search( | ||
r"http://(\d{1,3}(?:\.\d{1,3}){3}):(\d+)", backend_aware_bucketfs_params["url"] | ||
) | ||
url = infer_path( | ||
bucketfs_host=host_port.group(1), | ||
bucketfs_port=int(host_port.group(2)), | ||
bucketfs_name=backend_aware_bucketfs_params["service_name"], | ||
bucket=backend_aware_bucketfs_params["bucket_name"], | ||
bucketfs_user=backend_aware_bucketfs_params["username"], | ||
bucketfs_password=backend_aware_bucketfs_params["password"], | ||
path_in_bucket="onpremtest/", | ||
bucketfs_use_https=backend_aware_bucketfs_params["verify"], | ||
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. This will fail if a backend other than onprem is selected. Compare
all in pytest-plugins/pytest-backend 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 - please excuse me. I now replaced fixture |
||
) | ||
assert isinstance(url, bfs.path.BucketPath) | ||
assert backend_aware_bucketfs_params["url"] == url._bucket_api._service | ||
assert ( | ||
backend_aware_bucketfs_params["service_name"] == url._bucket_api._service_name | ||
) | ||
assert backend_aware_bucketfs_params["bucket_name"] == url._bucket_api._name | ||
assert "onpremtest" == str(url._path) | ||
|
||
|
||
@pytest.fixture | ||
def require_saas_params(backend_aware_saas_bucketfs_params, use_saas): | ||
if not use_saas: | ||
pytest.skip("Skipped as SaaS backend is not selected") | ||
return backend_aware_saas_bucketfs_params | ||
|
||
|
||
def test_infer_path_saas(require_saas_params): | ||
""" | ||
Creates the SaasBucket with fixture details realted to Saas and validates it. | ||
""" | ||
if backend != "saas": | ||
pytest.skip("The test runs only with SaaS database") | ||
url = infer_path( | ||
saas_url=saas_host, | ||
saas_account_id=saas_account_id, | ||
saas_database_id=backend_aware_saas_database_id, | ||
saas_token=saas_pat, | ||
path_in_bucket="saastest/", | ||
) | ||
assert isinstance(url, bfs.path.BucketPath) | ||
assert saas_host == url._bucket_api._url | ||
assert saas_account_id == url._bucket_api._account_id | ||
assert backend_aware_saas_database_id == url._bucket_api._database_id | ||
assert saas_pat == url._bucket_api._pat | ||
assert "saastest" in str(url._path) | ||
|
||
|
||
def test_infer_path_and_write( | ||
backend, | ||
backend_aware_bucketfs_params, | ||
children_poem, | ||
saas_host, | ||
saas_pat, | ||
saas_account_id, | ||
backend_aware_saas_database_id, | ||
): | ||
""" | ||
Combines the onprem and saas path inference tests | ||
and validates the path by uploading and reading data. | ||
""" | ||
if backend == "saas": | ||
if ( | ||
not saas_host | ||
or not saas_pat | ||
or not saas_account_id | ||
or not backend_aware_saas_database_id | ||
): | ||
pytest.skip("Skipping SaaS test due to missing parameters.") | ||
# Infer SaaS path | ||
path = infer_path( | ||
saas_url=saas_host, | ||
saas_account_id=saas_account_id, | ||
saas_database_id=backend_aware_saas_database_id, | ||
saas_token=saas_pat, | ||
path_in_bucket="test/", | ||
) | ||
else: | ||
# On-prem inference, extract host/port as needed | ||
host_port = re.search( | ||
r"http://(\d{1,3}(?:\.\d{1,3}){3}):(\d+)", | ||
backend_aware_bucketfs_params["url"], | ||
) | ||
path = infer_path( | ||
bucketfs_host=host_port.group(1), | ||
bucketfs_port=host_port.group(2), | ||
jana-selva marked this conversation as resolved.
Show resolved
Hide resolved
|
||
bucketfs_name=backend_aware_bucketfs_params["service_name"], | ||
bucket=backend_aware_bucketfs_params["bucket_name"], | ||
bucketfs_user=backend_aware_bucketfs_params["username"], | ||
bucketfs_password=backend_aware_bucketfs_params["password"], | ||
path_in_bucket="test/", | ||
bucketfs_use_https=backend_aware_bucketfs_params["verify"], | ||
) | ||
# Actually try uploading | ||
write_path = path / "test_file.txt" | ||
write_path.write(children_poem) | ||
|
||
# Read it back for verification | ||
read_back = b"".join(write_path.read(20)) | ||
assert read_back == children_poem |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
from enum import Enum | ||
from unittest.mock import ( | ||
patch, | ||
) | ||
|
||
import pytest | ||
|
||
from exasol.bucketfs._path import ( | ||
infer_backend, | ||
infer_path, | ||
) | ||
|
||
|
||
class StorageBackend(Enum): | ||
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. As @ahsimb said: This probably could go to production code. 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. Or simply use StorageBackend, maybe making it public by importing in |
||
onprem = "onprem" | ||
saas = "saas" | ||
|
||
|
||
# Dummy PathLike | ||
PathLike = str | ||
|
||
|
||
# Dummy build_path | ||
def build_path(*args, **kwargs): | ||
return f"mocked_path_{args}_{kwargs}" | ||
|
||
|
||
# Let's start with infer_backend | ||
def test_infer_backend_onprem(): | ||
result = infer_backend( | ||
bucketfs_host="host", | ||
bucketfs_port=123, | ||
bucketfs_name="bfs", | ||
bucket="mybucket", | ||
bucketfs_user="user", | ||
bucketfs_password="pw", | ||
) | ||
assert result == "onprem" | ||
|
||
|
||
def test_infer_backend_saas_with_id(): | ||
result = infer_backend( | ||
saas_url="https://api", | ||
saas_account_id="acct", | ||
saas_database_id="dbid", | ||
saas_token="token", | ||
) | ||
assert result == "saas" | ||
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. Compare to the enum |
||
|
||
|
||
def test_infer_backend_saas_with_name(): | ||
result = infer_backend( | ||
saas_url="https://api", | ||
saas_account_id="acct", | ||
saas_database_name="dbname", | ||
saas_token="token", | ||
) | ||
assert result == "saas" | ||
|
||
|
||
def test_infer_backend_missing_fields(): | ||
with pytest.raises(ValueError, match="Insufficient parameters"): | ||
infer_backend(bucketfs_host="host") # incomplete | ||
|
||
|
||
def test_infer_backend_no_fields(): | ||
with pytest.raises(ValueError): | ||
infer_backend() | ||
|
||
|
||
@patch("exasol.bucketfs._path.build_path", side_effect=build_path) | ||
def test_infer_path_onprem_with_ssl_ca(mock_build): | ||
# Should pass ssl_trusted_ca as argument verify to exasol.bucketfs._path.build_path() | ||
|
||
result = infer_path( | ||
bucketfs_host="host", | ||
bucketfs_port=123, | ||
bucketfs_name="bfs", | ||
bucket="mybucket", | ||
bucketfs_user="user", | ||
bucketfs_password="pw", | ||
ssl_trusted_ca="ca_cert", | ||
) | ||
called_args = mock_build.call_args[1] | ||
assert called_args["verify"] == "ca_cert" |
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.
Sorry, for a late comment. I think it might be useful to also include the Mounted BucketFS path here (and in the
infer_backend
). This backend is just a file path that can be used by either a UDF or for testing. Seebuild_path
.The ValueError would probably be "Unsupported backend". Insufficient parameters would be thrown by the
infer_backend
. If it was happy with the parameters, this must be some kind of a new backend, this function is unaware of or really doesn't want to support. This is a very small thing of course.