From 5d65db32e92bcfd1d63d3e4f00032ad1c8896719 Mon Sep 17 00:00:00 2001 From: Ethan Ho Date: Mon, 4 Dec 2023 10:04:13 -0600 Subject: [PATCH 1/6] First pass at migrating test_s3 --- tests/schema_external.py | 89 ++++++++++++++++++++++++++++++ tests/test_s3.py | 116 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 205 insertions(+) create mode 100644 tests/schema_external.py create mode 100644 tests/test_s3.py diff --git a/tests/schema_external.py b/tests/schema_external.py new file mode 100644 index 000000000..7702772fa --- /dev/null +++ b/tests/schema_external.py @@ -0,0 +1,89 @@ +""" +A schema for testing external attributes +""" + +import tempfile +import inspect +import datajoint as dj +from . import PREFIX, CONN_INFO, S3_CONN_INFO +import numpy as np + + +class Simple(dj.Manual): + definition = """ + simple : int + --- + item : blob@local + """ + + +class SimpleRemote(dj.Manual): + definition = """ + simple : int + --- + item : blob@share + """ + + +class Seed(dj.Lookup): + definition = """ + seed : int + """ + contents = zip(range(4)) + + +class Dimension(dj.Lookup): + definition = """ + dim : int + --- + dimensions : blob + """ + contents = ([0, [100, 50]], [1, [3, 4, 8, 6]]) + + +class Image(dj.Computed): + definition = """ + # table for storing + -> Seed + -> Dimension + ---- + img : blob@share # objects are stored as specified by dj.config['stores']['share'] + neg : blob@local # objects are stored as specified by dj.config['stores']['local'] + """ + + def make(self, key): + np.random.seed(key["seed"]) + img = np.random.rand(*(Dimension() & key).fetch1("dimensions")) + self.insert1(dict(key, img=img, neg=-img.astype(np.float32))) + + +class Attach(dj.Manual): + definition = """ + # table for storing attachments + attach : int + ---- + img : attach@share # attachments are stored as specified by: dj.config['stores']['raw'] + txt : attach # attachments are stored directly in the database + """ + + +class Filepath(dj.Manual): + definition = """ + # table for file management + fnum : int # test comment containing : + --- + img : filepath@repo # managed files + """ + + +class FilepathS3(dj.Manual): + definition = """ + # table for file management + fnum : int + --- + img : filepath@repo-s3 # managed files + """ + + +LOCALS_EXTERNAL= {k: v for k, v in locals().items() if inspect.isclass(v)} +__all__ = list(LOCALS_EXTERNAL) diff --git a/tests/test_s3.py b/tests/test_s3.py new file mode 100644 index 000000000..7173f7650 --- /dev/null +++ b/tests/test_s3.py @@ -0,0 +1,116 @@ +import pytest +import urllib3 +import certifi +from nose.tools import assert_true, raises +from .schema_external import SimpleRemote +from datajoint.errors import DataJointError +from datajoint.hash import uuid_from_buffer +from datajoint.blob import pack +from . import S3_CONN_INFO +from minio import Minio + +@pytest.fixture(scope='module') +def http_client(): + http_client = urllib3.PoolManager( + timeout=30, + cert_reqs="CERT_REQUIRED", + ca_certs=certifi.where(), + retries=urllib3.Retry( + total=3, backoff_factor=0.2, status_forcelist=[500, 502, 503, 504] + ), + ) + return http_client + + +@pytest.fixture(scope='module') +def minio_client(http_client): + # Initialize minioClient with an endpoint and access/secret keys. + minio_client = Minio( + S3_CONN_INFO["endpoint"], + access_key=S3_CONN_INFO["access_key"], + secret_key=S3_CONN_INFO["secret_key"], + secure=True, + http_client=http_client, + ) + return minio_client + + +@pytest.fixture(scope='session') +def stores_config(): + stores_config = { + "raw": dict(protocol="file", location=tempfile.mkdtemp()), + "repo": dict( + stage=tempfile.mkdtemp(), protocol="file", location=tempfile.mkdtemp() + ), + "repo-s3": dict( + S3_CONN_INFO, protocol="s3", location="dj/repo", stage=tempfile.mkdtemp() + ), + "local": dict(protocol="file", location=tempfile.mkdtemp(), subfolding=(1, 1)), + "share": dict( + S3_CONN_INFO, protocol="s3", location="dj/store/repo", subfolding=(2, 4) + ), + } + return stores_config + + +@pytest.fixture +def schema_ext(connection_test, stores_config, enable_filepath_feature): + schema = dj.Schema(PREFIX + "_extern", context=LOCALS_EXTERNAL, connection=connection_test) + dj.config["stores"] = stores_config + dj.config["cache"] = tempfile.mkdtemp() + + schema(Simple) + schema(SimpleRemote) + schema(Seed) + schema(Dimension) + schema(Image) + schema(Attach) + + # dj.errors._switch_filepath_types(True) + schema(Filepath) + schema(FilepathS3) + # dj.errors._switch_filepath_types(False) + yield schema + schema.drop() + + +class TestS3: + def test_connection(self, http_client, minio_client): + assert minio_client.bucket_exists(S3_CONN_INFO["bucket"]) + + def test_connection_secure(self, minio_client): + assert minio_client.bucket_exists(S3_CONN_INFO["bucket"]) + + def test_remove_object_exception(self): + # TODO: mv to failing block + with pytest.raises(DataJointError): + # https://github.com/datajoint/datajoint-python/issues/952 + + # Insert some test data and remove it so that the external table is populated + test = [1, [1, 2, 3]] + SimpleRemote.insert1(test) + SimpleRemote.delete() + + # Save the old external table minio client + old_client = schema.external["share"].s3.client + + # Apply our new minio client which has a user that does not exist + schema.external["share"].s3.client = Minio( + S3_CONN_INFO["endpoint"], + access_key="jeffjeff", + secret_key="jeffjeff", + secure=False, + ) + + # This method returns a list of errors + error_list = schema.external["share"].delete( + delete_external_files=True, errors_as_string=False + ) + + # Teardown + schema.external["share"].s3.client = old_client + schema.external["share"].delete(delete_external_files=True) + + # Raise the error we want if the error matches the expected uuid + if str(error_list[0][0]) == str(uuid_from_buffer(pack(test[1]))): + raise error_list[0][2] From 874c9cba2323be97df01a8d927bb029422302690 Mon Sep 17 00:00:00 2001 From: Ethan Ho Date: Tue, 5 Dec 2023 10:00:57 -0600 Subject: [PATCH 2/6] test_s3 uses bucket setup fixtures --- tests/conftest.py | 5 +++-- tests/test_s3.py | 25 ------------------------- 2 files changed, 3 insertions(+), 27 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 43a336254..fa51bb8a0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -269,6 +269,7 @@ def http_client(): @pytest.fixture(scope="session") def minio_client_bare(http_client): + """Initialize MinIO with an endpoint and access/secret keys.""" client = minio.Minio( S3_CONN_INFO["endpoint"], access_key=S3_CONN_INFO["access_key"], @@ -281,8 +282,8 @@ def minio_client_bare(http_client): @pytest.fixture(scope="session") def minio_client(minio_client_bare): - """Initialize MinIO with an endpoint and access/secret keys.""" - # Bootstrap MinIO bucket + """Initialize a MinIO client and create buckets for testing session.""" + # Setup MinIO bucket aws_region = "us-east-1" try: minio_client_bare.make_bucket(S3_CONN_INFO["bucket"], location=aws_region) diff --git a/tests/test_s3.py b/tests/test_s3.py index 7173f7650..b8fa0b958 100644 --- a/tests/test_s3.py +++ b/tests/test_s3.py @@ -9,31 +9,6 @@ from . import S3_CONN_INFO from minio import Minio -@pytest.fixture(scope='module') -def http_client(): - http_client = urllib3.PoolManager( - timeout=30, - cert_reqs="CERT_REQUIRED", - ca_certs=certifi.where(), - retries=urllib3.Retry( - total=3, backoff_factor=0.2, status_forcelist=[500, 502, 503, 504] - ), - ) - return http_client - - -@pytest.fixture(scope='module') -def minio_client(http_client): - # Initialize minioClient with an endpoint and access/secret keys. - minio_client = Minio( - S3_CONN_INFO["endpoint"], - access_key=S3_CONN_INFO["access_key"], - secret_key=S3_CONN_INFO["secret_key"], - secure=True, - http_client=http_client, - ) - return minio_client - @pytest.fixture(scope='session') def stores_config(): From 5568954548e60064c8e20956615d9f1076f258a0 Mon Sep 17 00:00:00 2001 From: Ethan Ho Date: Tue, 5 Dec 2023 10:06:56 -0600 Subject: [PATCH 3/6] Format with black --- tests/schema_external.py | 2 +- tests/test_s3.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/schema_external.py b/tests/schema_external.py index 7702772fa..294ecb070 100644 --- a/tests/schema_external.py +++ b/tests/schema_external.py @@ -85,5 +85,5 @@ class FilepathS3(dj.Manual): """ -LOCALS_EXTERNAL= {k: v for k, v in locals().items() if inspect.isclass(v)} +LOCALS_EXTERNAL = {k: v for k, v in locals().items() if inspect.isclass(v)} __all__ = list(LOCALS_EXTERNAL) diff --git a/tests/test_s3.py b/tests/test_s3.py index b8fa0b958..b2add2695 100644 --- a/tests/test_s3.py +++ b/tests/test_s3.py @@ -10,7 +10,7 @@ from minio import Minio -@pytest.fixture(scope='session') +@pytest.fixture(scope="session") def stores_config(): stores_config = { "raw": dict(protocol="file", location=tempfile.mkdtemp()), @@ -30,7 +30,9 @@ def stores_config(): @pytest.fixture def schema_ext(connection_test, stores_config, enable_filepath_feature): - schema = dj.Schema(PREFIX + "_extern", context=LOCALS_EXTERNAL, connection=connection_test) + schema = dj.Schema( + PREFIX + "_extern", context=LOCALS_EXTERNAL, connection=connection_test + ) dj.config["stores"] = stores_config dj.config["cache"] = tempfile.mkdtemp() From f993076e8c9f1a2f7ac8f2b4cedf297cbccf1938 Mon Sep 17 00:00:00 2001 From: Ethan Ho Date: Tue, 5 Dec 2023 12:17:56 -0600 Subject: [PATCH 4/6] Change scope of raises block --- tests/test_s3.py | 57 ++++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/tests/test_s3.py b/tests/test_s3.py index b2add2695..829ec104e 100644 --- a/tests/test_s3.py +++ b/tests/test_s3.py @@ -58,36 +58,35 @@ def test_connection(self, http_client, minio_client): def test_connection_secure(self, minio_client): assert minio_client.bucket_exists(S3_CONN_INFO["bucket"]) - def test_remove_object_exception(self): - # TODO: mv to failing block - with pytest.raises(DataJointError): - # https://github.com/datajoint/datajoint-python/issues/952 - - # Insert some test data and remove it so that the external table is populated - test = [1, [1, 2, 3]] - SimpleRemote.insert1(test) - SimpleRemote.delete() - - # Save the old external table minio client - old_client = schema.external["share"].s3.client - - # Apply our new minio client which has a user that does not exist - schema.external["share"].s3.client = Minio( - S3_CONN_INFO["endpoint"], - access_key="jeffjeff", - secret_key="jeffjeff", - secure=False, - ) - - # This method returns a list of errors - error_list = schema.external["share"].delete( - delete_external_files=True, errors_as_string=False - ) - - # Teardown - schema.external["share"].s3.client = old_client - schema.external["share"].delete(delete_external_files=True) + def test_remove_object_exception(self, schema_ext): + # https://github.com/datajoint/datajoint-python/issues/952 + + # Insert some test data and remove it so that the external table is populated + test = [1, [1, 2, 3]] + SimpleRemote.insert1(test) + SimpleRemote.delete() + + # Save the old external table minio client + old_client = schema_ext.external["share"].s3.client + + # Apply our new minio client which has a user that does not exist + schema_ext.external["share"].s3.client = Minio( + S3_CONN_INFO["endpoint"], + access_key="jeffjeff", + secret_key="jeffjeff", + secure=False, + ) + + # This method returns a list of errors + error_list = schema_ext.external["share"].delete( + delete_external_files=True, errors_as_string=False + ) + + # Teardown + schema_ext.external["share"].s3.client = old_client + schema_ext.external["share"].delete(delete_external_files=True) + with pytest.raises(DataJointError): # Raise the error we want if the error matches the expected uuid if str(error_list[0][0]) == str(uuid_from_buffer(pack(test[1]))): raise error_list[0][2] From 3cd99f1f01bfc9cf6df3b0a9369d3cc494e1d9aa Mon Sep 17 00:00:00 2001 From: Ethan Ho Date: Tue, 5 Dec 2023 12:19:36 -0600 Subject: [PATCH 5/6] Move schema_ext to conftest --- tests/conftest.py | 65 ++++++++++++++++++++++++++++---- tests/test_adapted_attributes.py | 15 ++------ tests/test_s3.py | 42 --------------------- 3 files changed, 61 insertions(+), 61 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 4f70c5f3a..fedaa20c0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -23,13 +23,7 @@ schema_simple, schema_advanced, schema_adapted, - PREFIX, - CONN_INFO, - S3_CONN_INFO, - schema, - schema_simple, - schema_advanced, - schema_adapted, + schema_external, ) @@ -45,6 +39,20 @@ def monkeymodule(): yield mp +@pytest.fixture +def enable_adapted_types(monkeypatch): + monkeypatch.setenv(ADAPTED_TYPE_SWITCH, "TRUE") + yield + monkeypatch.delenv(ADAPTED_TYPE_SWITCH, raising=True) + + +@pytest.fixture +def enable_filepath_feature(monkeypatch): + monkeypatch.setenv(FILEPATH_FEATURE_SWITCH, "TRUE") + yield + monkeypatch.delenv(FILEPATH_FEATURE_SWITCH, raising=True) + + @pytest.fixture(scope="session") def connection_root_bare(): connection = dj.Connection( @@ -168,6 +176,24 @@ def connection_test(connection_root): connection.close() +@pytest.fixture(scope="session") +def stores_config(): + stores_config = { + "raw": dict(protocol="file", location=tempfile.mkdtemp()), + "repo": dict( + stage=tempfile.mkdtemp(), protocol="file", location=tempfile.mkdtemp() + ), + "repo-s3": dict( + S3_CONN_INFO, protocol="s3", location="dj/repo", stage=tempfile.mkdtemp() + ), + "local": dict(protocol="file", location=tempfile.mkdtemp(), subfolding=(1, 1)), + "share": dict( + S3_CONN_INFO, protocol="s3", location="dj/store/repo", subfolding=(2, 4) + ), + } + return stores_config + + @pytest.fixture def schema_any(connection_test): schema_any = dj.Schema( @@ -261,6 +287,31 @@ def schema_adv(connection_test): schema.drop() +@pytest.fixture +def schema_ext(connection_test, stores_config, enable_filepath_feature): + schema = dj.Schema( + PREFIX + "_extern", + context=schema_external.LOCALS_EXTERNAL, + connection=connection_test, + ) + dj.config["stores"] = stores_config + dj.config["cache"] = tempfile.mkdtemp() + + schema(schema_external.Simple) + schema(schema_external.SimpleRemote) + schema(schema_external.Seed) + schema(schema_external.Dimension) + schema(schema_external.Image) + schema(schema_external.Attach) + + # dj.errors._switch_filepath_types(True) + schema(schema_external.Filepath) + schema(schema_external.FilepathS3) + # dj.errors._switch_filepath_types(False) + yield schema + schema.drop() + + @pytest.fixture(scope="session") def http_client(): # Initialize httpClient with relevant timeout. diff --git a/tests/test_adapted_attributes.py b/tests/test_adapted_attributes.py index 61166f68f..cf06575c6 100644 --- a/tests/test_adapted_attributes.py +++ b/tests/test_adapted_attributes.py @@ -2,7 +2,6 @@ import pytest import tempfile import datajoint as dj -from datajoint.errors import ADAPTED_TYPE_SWITCH, FILEPATH_FEATURE_SWITCH import networkx as nx from itertools import zip_longest from . import schema_adapted @@ -20,17 +19,9 @@ def adapted_graph_instance(): @pytest.fixture -def enable_adapted_types(monkeypatch): - monkeypatch.setenv(ADAPTED_TYPE_SWITCH, "TRUE") - yield - monkeypatch.delenv(ADAPTED_TYPE_SWITCH, raising=True) - - -@pytest.fixture -def enable_filepath_feature(monkeypatch): - monkeypatch.setenv(FILEPATH_FEATURE_SWITCH, "TRUE") - yield - monkeypatch.delenv(FILEPATH_FEATURE_SWITCH, raising=True) +def schema_name_custom_datatype(): + schema_name = PREFIX + "_test_custom_datatype" + return schema_name @pytest.fixture diff --git a/tests/test_s3.py b/tests/test_s3.py index 829ec104e..090d6acf0 100644 --- a/tests/test_s3.py +++ b/tests/test_s3.py @@ -1,7 +1,6 @@ import pytest import urllib3 import certifi -from nose.tools import assert_true, raises from .schema_external import SimpleRemote from datajoint.errors import DataJointError from datajoint.hash import uuid_from_buffer @@ -10,47 +9,6 @@ from minio import Minio -@pytest.fixture(scope="session") -def stores_config(): - stores_config = { - "raw": dict(protocol="file", location=tempfile.mkdtemp()), - "repo": dict( - stage=tempfile.mkdtemp(), protocol="file", location=tempfile.mkdtemp() - ), - "repo-s3": dict( - S3_CONN_INFO, protocol="s3", location="dj/repo", stage=tempfile.mkdtemp() - ), - "local": dict(protocol="file", location=tempfile.mkdtemp(), subfolding=(1, 1)), - "share": dict( - S3_CONN_INFO, protocol="s3", location="dj/store/repo", subfolding=(2, 4) - ), - } - return stores_config - - -@pytest.fixture -def schema_ext(connection_test, stores_config, enable_filepath_feature): - schema = dj.Schema( - PREFIX + "_extern", context=LOCALS_EXTERNAL, connection=connection_test - ) - dj.config["stores"] = stores_config - dj.config["cache"] = tempfile.mkdtemp() - - schema(Simple) - schema(SimpleRemote) - schema(Seed) - schema(Dimension) - schema(Image) - schema(Attach) - - # dj.errors._switch_filepath_types(True) - schema(Filepath) - schema(FilepathS3) - # dj.errors._switch_filepath_types(False) - yield schema - schema.drop() - - class TestS3: def test_connection(self, http_client, minio_client): assert minio_client.bucket_exists(S3_CONN_INFO["bucket"]) From 3b5047b346b66cf6a1f4c32ea9a3fa70ce574d42 Mon Sep 17 00:00:00 2001 From: Ethan Ho Date: Tue, 5 Dec 2023 12:47:38 -0600 Subject: [PATCH 6/6] Add @A-Baji suggestions --- tests/conftest.py | 3 --- tests/test_adapted_attributes.py | 6 ------ 2 files changed, 9 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index fedaa20c0..5a4858636 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -303,11 +303,8 @@ def schema_ext(connection_test, stores_config, enable_filepath_feature): schema(schema_external.Dimension) schema(schema_external.Image) schema(schema_external.Attach) - - # dj.errors._switch_filepath_types(True) schema(schema_external.Filepath) schema(schema_external.FilepathS3) - # dj.errors._switch_filepath_types(False) yield schema schema.drop() diff --git a/tests/test_adapted_attributes.py b/tests/test_adapted_attributes.py index a0af540c7..bbe8456f5 100644 --- a/tests/test_adapted_attributes.py +++ b/tests/test_adapted_attributes.py @@ -16,12 +16,6 @@ def adapted_graph_instance(): yield schema_adapted.GraphAdapter() -@pytest.fixture -def schema_name_custom_datatype(): - schema_name = PREFIX + "_test_custom_datatype" - return schema_name - - @pytest.fixture def schema_ad( connection_test,