From b7806a3c053eb022ea79999d70d9ad080e16ef95 Mon Sep 17 00:00:00 2001 From: Christoph Blessing <33834216+cblessing24@users.noreply.github.com> Date: Fri, 6 Oct 2023 13:27:34 +0200 Subject: [PATCH 1/4] Allow direct inserts into computed tables --- tests/functional/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py index dff0828..f6f323c 100644 --- a/tests/functional/conftest.py +++ b/tests/functional/conftest.py @@ -521,7 +521,7 @@ def _prepare_table(database, user, schema, table_cls, *, data=None, parts=None, parts = {} with dj_connection(database, user) as connection: dj.schema(schema, connection=connection, context=context)(table_cls) - table_cls().insert(data) + table_cls().insert(data, allow_direct_insert=True) for name, part_data in parts.items(): getattr(table_cls, name).insert(part_data) From 64a6959370cddfdb62eac2c0b9d8e49aa1ef4208 Mon Sep 17 00:00:00 2001 From: Christoph Blessing <33834216+cblessing24@users.noreply.github.com> Date: Fri, 6 Oct 2023 13:29:05 +0200 Subject: [PATCH 2/4] Test if part of computed gets correctly named --- tests/functional/test_creation.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/functional/test_creation.py b/tests/functional/test_creation.py index 1bf0a96..8bd7498 100644 --- a/tests/functional/test_creation.py +++ b/tests/functional/test_creation.py @@ -1,4 +1,5 @@ import datajoint as dj +import pytest from link import link @@ -47,3 +48,26 @@ def test_local_table_creation_from_source_table_that_uses_current_timestamp_defa "Outbound", schema_names["local"], )(type(source_table_name, (dj.Manual,), {})) + + +@pytest.mark.xfail() +def test_part_tables_of_computed_source_gets_created_with_correct_name( + prepare_link, create_table, prepare_table, databases, configured_environment, connection_config +): + schema_names, user_specs = prepare_link() + source_table_name = "Foo" + source_table = create_table( + source_table_name, dj.Computed, "foo: int", parts=[create_table("Bar", dj.Part, "-> master")] + ) + prepare_table(databases["source"], user_specs["source"], schema_names["source"], source_table) + with connection_config(databases["local"], user_specs["local"]), configured_environment( + user_specs["link"], schema_names["outbound"] + ): + local_table = link( + databases["source"].container.name, + schema_names["source"], + schema_names["outbound"], + "Outbound", + schema_names["local"], + )(type(source_table_name, tuple(), {})) + assert hasattr(local_table, "Bar") From 54b345bf7aa685d330d79796bf255df1f2d8d741 Mon Sep 17 00:00:00 2001 From: Christoph Blessing <33834216+cblessing24@users.noreply.github.com> Date: Fri, 6 Oct 2023 13:31:48 +0200 Subject: [PATCH 3/4] Fix part of computed getting incorrect name --- link/infrastructure/factory.py | 2 +- tests/functional/test_creation.py | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/link/infrastructure/factory.py b/link/infrastructure/factory.py index 0c595da..8a96317 100644 --- a/link/infrastructure/factory.py +++ b/link/infrastructure/factory.py @@ -94,7 +94,7 @@ def create_dj_table() -> dj.Table: if not child.table_name.startswith(parts().table_name + "__"): continue part_definition = child.describe(printout=False).replace(parts().full_table_name, "master") - part_definitions[dj.utils.to_camel_case(child.table_name.split("__")[1])] = part_definition + part_definitions[dj.utils.to_camel_case(child.table_name.split("__")[-1])] = part_definition for part_name, part_definition in part_definitions.items(): part_definitions[part_name] = replace_stores(part_definition, replacement_stores) part_tables: dict[str, type[dj.Part]] = {} diff --git a/tests/functional/test_creation.py b/tests/functional/test_creation.py index 8bd7498..8c0ea29 100644 --- a/tests/functional/test_creation.py +++ b/tests/functional/test_creation.py @@ -1,5 +1,4 @@ import datajoint as dj -import pytest from link import link @@ -50,7 +49,6 @@ def test_local_table_creation_from_source_table_that_uses_current_timestamp_defa )(type(source_table_name, (dj.Manual,), {})) -@pytest.mark.xfail() def test_part_tables_of_computed_source_gets_created_with_correct_name( prepare_link, create_table, prepare_table, databases, configured_environment, connection_config ): From 74df031555aad6c7408b8605b16632811bad54c4 Mon Sep 17 00:00:00 2001 From: Christoph Blessing <33834216+cblessing24@users.noreply.github.com> Date: Fri, 6 Oct 2023 13:33:49 +0200 Subject: [PATCH 4/4] Refactor functional tests to use actors --- tests/functional/conftest.py | 140 ++++++++++++++---------------- tests/functional/test_creation.py | 69 +++++++-------- tests/functional/test_deleting.py | 35 +++----- tests/functional/test_pulling.py | 22 ++--- 4 files changed, 113 insertions(+), 153 deletions(-) diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py index f6f323c..cc2dc7b 100644 --- a/tests/functional/conftest.py +++ b/tests/functional/conftest.py @@ -4,10 +4,9 @@ import pathlib from concurrent import futures from contextlib import contextmanager -from dataclasses import asdict, dataclass +from dataclasses import asdict, dataclass, field from random import choices from string import ascii_lowercase -from typing import Dict import datajoint as dj import docker @@ -59,7 +58,6 @@ class HealthCheckConfig: @dataclass(frozen=True) class DatabaseConfig: password: str # MYSQL root user password - users: Dict[str, UserConfig] schema_name: str @@ -71,6 +69,7 @@ class DatabaseSpec: @dataclass(frozen=True) class UserConfig: + host: str name: str password: str grants: list[str] @@ -104,35 +103,6 @@ def docker_client(): return docker.client.from_env() -@pytest.fixture(scope=SCOPE) -def create_user_configs(outbound_schema_name): - def _create_user_configs(schema_name): - return dict( - admin_user=UserConfig( - "admin_user", - "admin_user_password", - grants=[ - f"GRANT ALL PRIVILEGES ON `{outbound_schema_name}`.* TO '$name'@'%';", - ], - ), - end_user=UserConfig( - "end_user", - "end_user_password", - grants=[r"GRANT ALL PRIVILEGES ON `end_user\_%`.* TO '$name'@'%';"], - ), - dj_user=UserConfig( - "dj_user", - "dj_user_password", - grants=[ - f"GRANT SELECT, REFERENCES ON `{schema_name}`.* TO '$name'@'%';", - f"GRANT ALL PRIVILEGES ON `{outbound_schema_name}`.* TO '$name'@'%';", - ], - ), - ) - - return _create_user_configs - - @pytest.fixture(scope=SCOPE) def create_random_string(): def _create_random_string(length=6): @@ -147,7 +117,7 @@ def network(): @pytest.fixture(scope=SCOPE) -def get_db_spec(create_random_string, create_user_configs, network): +def get_db_spec(create_random_string, network): def _get_db_spec(name): schema_name = "end_user_schema" return DatabaseSpec( @@ -160,7 +130,6 @@ def _get_db_spec(name): ), DatabaseConfig( password=DATABASE_ROOT_PASSWORD, - users=create_user_configs(schema_name), schema_name=schema_name, ), ) @@ -188,13 +157,6 @@ def _get_minio_spec(name): return _get_minio_spec -@pytest.fixture(scope=SCOPE) -def outbound_schema_name(): - name = "outbound_schema" - os.environ["LINK_OUTBOUND"] = name - return name - - def get_runner_kwargs(docker_client, spec): common = dict( detach=True, @@ -235,20 +197,15 @@ def get_runner_kwargs(docker_client, spec): @pytest.fixture(scope=SCOPE) -def create_user_config(create_random_string): - def _create_user_config(grants): - name = create_random_string() - return UserConfig( - name=name, password=create_random_string(), grants=[grant.replace("$name", name) for grant in grants] - ) - - return _create_user_config - - -@pytest.fixture(scope=SCOPE) -def create_user(create_user_config): +def create_user(create_random_string): def _create_user(db_spec, grants): - config = create_user_config(grants) + user_name = create_random_string() + config = UserConfig( + host=db_spec.container.name, + name=user_name, + password=create_random_string(), + grants=[grant.replace("$name", user_name) for grant in grants], + ) with mysql_conn(db_spec) as connection: with connection.cursor() as cursor: cursor.execute(f"CREATE USER '{config.name}'@'%' IDENTIFIED BY '{config.password}';") @@ -347,8 +304,10 @@ def _get_store_spec(minio_spec, protocol="s3", port=9000): @pytest.fixture() def dj_connection(): @contextmanager - def _dj_connection(db_spec, user_spec): - connection = dj.Connection(db_spec.container.name, user_spec.name, user_spec.password) + def _dj_connection(): + connection = dj.Connection( + dj.config["database.host"], dj.config["database.user"], dj.config["database.password"] + ) try: yield connection finally: @@ -360,12 +319,12 @@ def _dj_connection(db_spec, user_spec): @pytest.fixture() def connection_config(): @contextmanager - def _connection_config(db_spec, user): + def _connection_config(actor): try: with dj.config( - database__host=db_spec.container.name, - database__user=user.name, - database__password=user.password, + database__host=actor.credentials.host, + database__user=actor.credentials.name, + database__password=actor.credentials.password, safemode=False, ): dj.conn(reset=True) @@ -449,16 +408,34 @@ def _temp_env_vars(**vars): return _temp_env_vars +@pytest.fixture() +def act_as(connection_config, temp_env_vars): + @contextmanager + def _act_as(actor): + with connection_config(actor), temp_env_vars(**actor.environment): + yield + + return _act_as + + @pytest.fixture() def configured_environment(temp_env_vars): @contextmanager - def _configured_environment(user_spec, schema_name): - with temp_env_vars(LINK_USER=user_spec.name, LINK_PASS=user_spec.password, LINK_OUTBOUND=schema_name): + def _configured_environment(actor, schema_name): + with temp_env_vars( + LINK_USER=actor.credentials.name, LINK_PASS=actor.credentials.password, LINK_OUTBOUND=schema_name + ): yield return _configured_environment +@dataclass(frozen=True) +class Actor: + credentials: UserConfig + environment: dict[str, str] = field(default_factory=dict) + + @pytest.fixture() def prepare_multiple_links(create_random_string, create_user, databases): def _prepare_multiple_links(n_local_schemas): @@ -468,24 +445,37 @@ def create_schema_names(): return names schema_names = create_schema_names() - user_specs = { - "admin": create_user(databases["source"], grants=["GRANT ALL PRIVILEGES ON *.* TO '$name'@'%';"]), - "source": create_user( - databases["source"], grants=[f"GRANT ALL PRIVILEGES ON `{schema_names['source']}`.* TO '$name'@'%';"] - ), - "local": create_user( - databases["local"], - grants=[f"GRANT ALL PRIVILEGES ON `{name}`.* TO '$name'@'%';" for name in schema_names["local"]], - ), - "link": create_user( + link_actor = Actor( + create_user( databases["source"], grants=[ f"GRANT SELECT, REFERENCES ON `{schema_names['source']}`.* TO '$name'@'%';", f"GRANT ALL PRIVILEGES ON `{schema_names['outbound']}`.* TO '$name'@'%';", ], + ) + ) + actors = { + "admin": Actor(create_user(databases["source"], grants=["GRANT ALL PRIVILEGES ON *.* TO '$name'@'%';"])), + "source": Actor( + create_user( + databases["source"], + grants=[f"GRANT ALL PRIVILEGES ON `{schema_names['source']}`.* TO '$name'@'%';"], + ) ), + "local": Actor( + create_user( + databases["local"], + grants=[f"GRANT ALL PRIVILEGES ON `{name}`.* TO '$name'@'%';" for name in schema_names["local"]], + ), + { + "LINK_USER": link_actor.credentials.name, + "LINK_PASS": link_actor.credentials.password, + "LINK_OUTBOUND": schema_names["outbound"], + }, + ), + "link": link_actor, } - return schema_names, user_specs + return schema_names, actors return _prepare_multiple_links @@ -514,12 +504,12 @@ def _create_table(name, tier, definition, *, parts=None): @pytest.fixture() def prepare_table(dj_connection): - def _prepare_table(database, user, schema, table_cls, *, data=None, parts=None, context=None): + def _prepare_table(schema, table_cls, *, data=None, parts=None, context=None): if data is None: data = [] if parts is None: parts = {} - with dj_connection(database, user) as connection: + with dj_connection() as connection: dj.schema(schema, connection=connection, context=context)(table_cls) table_cls().insert(data, allow_direct_insert=True) for name, part_data in parts.items(): diff --git a/tests/functional/test_creation.py b/tests/functional/test_creation.py index 8c0ea29..676ed1f 100644 --- a/tests/functional/test_creation.py +++ b/tests/functional/test_creation.py @@ -4,65 +4,56 @@ def test_local_table_creation_from_source_table_that_has_parent_raises_no_error( - prepare_link, create_table, prepare_table, databases, configured_environment, connection_config + prepare_link, create_table, prepare_table, act_as ): - schema_names, user_specs = prepare_link() - source_table_parent = create_table("Foo", dj.Manual, "foo: int") - prepare_table(databases["source"], user_specs["source"], schema_names["source"], source_table_parent) - source_table_name = "Bar" - source_table = create_table(source_table_name, dj.Manual, "-> source_table_parent") - prepare_table( - databases["source"], - user_specs["source"], - schema_names["source"], - source_table, - context={"source_table_parent": source_table_parent}, - ) - with connection_config(databases["local"], user_specs["local"]), configured_environment( - user_specs["link"], schema_names["outbound"] - ): + schema_names, actors = prepare_link() + with act_as(actors["source"]): + source_table_parent = create_table("Foo", dj.Manual, "foo: int") + prepare_table(schema_names["source"], source_table_parent) + source_table_name = "Bar" + source_table = create_table(source_table_name, dj.Manual, "-> source_table_parent") + prepare_table(schema_names["source"], source_table, context={"source_table_parent": source_table_parent}) + with act_as(actors["local"]): link( - databases["source"].container.name, + actors["source"].credentials.host, schema_names["source"], schema_names["outbound"], "Outbound", schema_names["local"], - )(type(source_table_name, (dj.Manual,), {})) + )(type(source_table_name, tuple(), {})) def test_local_table_creation_from_source_table_that_uses_current_timestamp_default_raises_no_error( - prepare_link, create_table, prepare_table, databases, configured_environment, connection_config + prepare_link, create_table, prepare_table, act_as ): - schema_names, user_specs = prepare_link() - source_table_name = "Foo" - source_table = create_table(source_table_name, dj.Manual, "foo = CURRENT_TIMESTAMP : timestamp") - prepare_table(databases["source"], user_specs["source"], schema_names["source"], source_table) - with connection_config(databases["local"], user_specs["local"]), configured_environment( - user_specs["link"], schema_names["outbound"] - ): + schema_names, actors = prepare_link() + with act_as(actors["source"]): + source_table_name = "Foo" + source_table = create_table(source_table_name, dj.Manual, "foo = CURRENT_TIMESTAMP : timestamp") + prepare_table(schema_names["source"], source_table) + with act_as(actors["local"]): link( - databases["source"].container.name, + actors["source"].credentials.host, schema_names["source"], schema_names["outbound"], "Outbound", schema_names["local"], - )(type(source_table_name, (dj.Manual,), {})) + )(type(source_table_name, tuple(), {})) def test_part_tables_of_computed_source_gets_created_with_correct_name( - prepare_link, create_table, prepare_table, databases, configured_environment, connection_config + prepare_link, create_table, prepare_table, act_as ): - schema_names, user_specs = prepare_link() - source_table_name = "Foo" - source_table = create_table( - source_table_name, dj.Computed, "foo: int", parts=[create_table("Bar", dj.Part, "-> master")] - ) - prepare_table(databases["source"], user_specs["source"], schema_names["source"], source_table) - with connection_config(databases["local"], user_specs["local"]), configured_environment( - user_specs["link"], schema_names["outbound"] - ): + schema_names, actors = prepare_link() + with act_as(actors["source"]): + source_table_name = "Foo" + source_table = create_table( + source_table_name, dj.Computed, "foo: int", parts=[create_table("Bar", dj.Part, "-> master")] + ) + prepare_table(schema_names["source"], source_table) + with act_as(actors["local"]): local_table = link( - databases["source"].container.name, + actors["source"].credentials.host, schema_names["source"], schema_names["outbound"], "Outbound", diff --git a/tests/functional/test_deleting.py b/tests/functional/test_deleting.py index 1119170..1b12b4a 100644 --- a/tests/functional/test_deleting.py +++ b/tests/functional/test_deleting.py @@ -3,27 +3,18 @@ from link import link -def test_deleting( - prepare_link, prepare_table, dj_connection, databases, connection_config, configured_environment, create_table -): +def test_deleting(prepare_link, prepare_table, act_as, create_table): data = [{"foo": 1}, {"foo": 2}, {"foo": 3}] expected = [{"foo": 2}, {"foo": 3}] - schema_names, user_specs = prepare_link() - - table_cls = create_table("Foo", dj.Manual, "foo: int") - prepare_table( - databases["source"], - user_specs["source"], - schema_names["source"], - table_cls, - data=data, - ) - - with connection_config(databases["local"], user_specs["local"]), configured_environment( - user_specs["link"], schema_names["outbound"] - ): + schema_names, actors = prepare_link() + + with act_as(actors["source"]): + table_cls = create_table("Foo", dj.Manual, "foo: int") + prepare_table(schema_names["source"], table_cls, data=data) + + with act_as(actors["local"]): local_table_cls = link( - databases["source"].container.name, + actors["source"].credentials.host, schema_names["source"], schema_names["outbound"], "Outbound", @@ -31,18 +22,16 @@ def test_deleting( )(type("Foo", tuple(), {})) local_table_cls().source.pull() - with dj_connection(databases["source"], user_specs["admin"]) as connection: + with act_as(actors["admin"]): table_classes = {} - dj.schema(schema_names["outbound"], connection=connection).spawn_missing_classes(context=table_classes) + dj.schema(schema_names["outbound"]).spawn_missing_classes(context=table_classes) outbound_table_cls = table_classes["Outbound"] row = (outbound_table_cls & {"foo": 1}).fetch1() (outbound_table_cls() & row).delete_quick() row["is_flagged"] = "TRUE" outbound_table_cls().insert1(row) - with connection_config(databases["local"], user_specs["local"]), configured_environment( - user_specs["link"], schema_names["outbound"] - ): + with act_as(actors["local"]): (local_table_cls() & local_table_cls().source.flagged).delete() assert local_table_cls().fetch(as_dict=True) == expected diff --git a/tests/functional/test_pulling.py b/tests/functional/test_pulling.py index c647eca..020af4f 100644 --- a/tests/functional/test_pulling.py +++ b/tests/functional/test_pulling.py @@ -12,12 +12,10 @@ def test_pulling( prepare_table, tmpdir, create_table, - connection_config, temp_dj_store_config, temp_store, - databases, minios, - configured_environment, + act_as, ): @functools.lru_cache() def create_random_binary_file(seed, *, n_bytes=1024): @@ -45,7 +43,7 @@ def create_random_binary_file(seed, *, n_bytes=1024): "Part2": data_parts["Part2"][:2], } - schema_names, user_specs = prepare_link() + schema_names, actors = prepare_link() with temp_store(minios["source"]) as source_store_spec, temp_store(minios["local"]) as local_store_spec: part_table_definitions = { "Part1": f"-> master\nbaz: int\n---\negg: attach@{source_store_spec.name}", @@ -66,20 +64,12 @@ def create_random_table_name(): f"foo: int\n---\nbar: attach@{source_store_spec.name}", parts=part_table_classes, ) - prepare_table( - databases["source"], - user_specs["source"], - schema_names["source"], - table_cls, - data=data, - parts=data_parts, - ) + with act_as(actors["source"]): + prepare_table(schema_names["source"], table_cls, data=data, parts=data_parts) - with connection_config(databases["local"], user_specs["local"]), configured_environment( - user_specs["link"], schema_names["outbound"] - ), temp_dj_store_config([source_store_spec, local_store_spec]): + with act_as(actors["local"]), temp_dj_store_config([source_store_spec, local_store_spec]): local_table_cls = link( - databases["source"].container.name, + actors["source"].credentials.host, schema_names["source"], schema_names["outbound"], "Outbound",