diff --git a/arkindex_worker/reporting.py b/arkindex_worker/reporting.py index b9ee03ef465795cefc377fdaf9f300f3f3d252b1..e30f8f954bb84bad26489f72aa24e038a45aaf66 100644 --- a/arkindex_worker/reporting.py +++ b/arkindex_worker/reporting.py @@ -26,13 +26,13 @@ class Reporter(object): self, name: Optional[str] = "Unknown worker", slug: Optional[str] = "unknown-slug", - version: Optional[str] = None, + run: Optional[str] = None, **kwargs, ): self.report_data = { "name": name, "slug": slug, - "version": version, + "run": run, "started": datetime.utcnow().isoformat(), "elements": {}, } diff --git a/arkindex_worker/worker/__init__.py b/arkindex_worker/worker/__init__.py index 7ff2f70e4fe6a35e06c773615db03530475fec45..a4d5af9c6cbd9599c3f799ff09c442735611dfdf 100644 --- a/arkindex_worker/worker/__init__.py +++ b/arkindex_worker/worker/__init__.py @@ -158,7 +158,7 @@ class ElementsWorker( # Add report concerning elements self.report = Reporter( - **self.worker_details, version=getattr(self, "worker_version_id", None) + **self.worker_details, run=getattr(self, "worker_run_id", None) ) def run(self): diff --git a/arkindex_worker/worker/base.py b/arkindex_worker/worker/base.py index 4a2a93e8e6eaff35fc83aec270e3f80f234b48e5..4e17544a52f4040e489fe4d185fca450b15edfac 100644 --- a/arkindex_worker/worker/base.py +++ b/arkindex_worker/worker/base.py @@ -165,6 +165,10 @@ class BaseWorker(object): """ return self.args.dev or self.worker_run_id is None + @property + def worker_version_id(self): + raise DeprecationWarning("`worker_version_id` usage is deprecated") + def setup_api_client(self): """ Create an ArkindexClient to make API requests towards Arkindex instances. @@ -234,8 +238,10 @@ class BaseWorker(object): # Load worker version information worker_version = worker_run["worker_version"] - # Store worker version id - self.worker_version_id = worker_version["id"] + # Store worker version id in hidden attribute + # We only need this attribute till Worker Activities are linked + # to WorkerRuns instead of WorkerVersions + self._worker_version_id = worker_version["id"] self.worker_details = worker_version["worker"] logger.info( diff --git a/arkindex_worker/worker/element.py b/arkindex_worker/worker/element.py index a64002d5cbc70a30d9cb7154ec93d4208917be66..b9e3c2ae3234518baa00c4d04563222b66019dc6 100644 --- a/arkindex_worker/worker/element.py +++ b/arkindex_worker/worker/element.py @@ -3,6 +3,7 @@ ElementsWorker methods for elements and element types. """ from typing import Dict, Iterable, List, NamedTuple, Optional, Union +from warnings import warn from peewee import IntegrityError @@ -359,6 +360,13 @@ class ElementMixin(object): """ List children of an element. + Warns: + ---- + The following parameters are **deprecated**: + + - `transcription_worker_version` in favor of `transcription_worker_run` + - `worker_version` in favor of `worker_run` + :param element: Parent element to find children of. :param folder: Restrict to or exclude elements with folder types. This parameter is not supported when caching is enabled. @@ -366,7 +374,7 @@ class ElementMixin(object): This parameter is not supported when caching is enabled. :param recursive: Look for elements recursively (grand-children, etc.) This parameter is not supported when caching is enabled. - :param transcription_worker_version: Restrict to elements that have a transcription created by a worker version with this UUID. + :param transcription_worker_version: **Deprecated** Restrict to elements that have a transcription created by a worker version with this UUID. This parameter is not supported when caching is enabled. :param transcription_worker_run: Restrict to elements that have a transcription created by a worker run with this UUID. This parameter is not supported when caching is enabled. @@ -384,7 +392,7 @@ class ElementMixin(object): :param with_zone: Include the ``zone`` attribute in the response, holding the element's image and polygon. This parameter is not supported when caching is enabled. - :param worker_version: Restrict to elements created by a worker version with this UUID. + :param worker_version: **Deprecated** Restrict to elements created by a worker version with this UUID. :param worker_run: Restrict to elements created by a worker run with this UUID. :return: An iterable of dicts from the ``ListElementChildren`` API endpoint, or an iterable of [CachedElement][arkindex_worker.cache.CachedElement] when caching is enabled. @@ -403,6 +411,11 @@ class ElementMixin(object): assert isinstance(recursive, bool), "recursive should be of type bool" query_params["recursive"] = recursive if transcription_worker_version is not None: + warn( + "`transcription_worker_version` usage is deprecated. Consider using `transcription_worker_run` instead.", + DeprecationWarning, + stacklevel=2, + ) assert isinstance( transcription_worker_version, (str, bool) ), "transcription_worker_version should be of type str or bool" @@ -443,6 +456,11 @@ class ElementMixin(object): assert isinstance(with_zone, bool), "with_zone should be of type bool" query_params["with_zone"] = with_zone if worker_version is not None: + warn( + "`worker_version` usage is deprecated. Consider using `worker_run` instead.", + DeprecationWarning, + stacklevel=2, + ) assert isinstance( worker_version, (str, bool) ), "worker_version should be of type str or bool" diff --git a/arkindex_worker/worker/version.py b/arkindex_worker/worker/version.py index 85d6f8d72b323c01478b68b509b4113cdca0cb86..3af9153a6807a45f6526a3deea74576f7f085feb 100644 --- a/arkindex_worker/worker/version.py +++ b/arkindex_worker/worker/version.py @@ -3,8 +3,25 @@ ElementsWorker methods for worker versions. """ +import functools +from warnings import warn + + +def worker_version_deprecation(func): + """ + Return shortly in case the is_read_only property is evaluated to True + """ + + @functools.wraps(func) + def wrapper(self, *args, **kwargs): + warn("WorkerVersion usage is deprecated.", DeprecationWarning, stacklevel=2) + return func(self, *args, **kwargs) + + return wrapper + class WorkerVersionMixin(object): + @worker_version_deprecation def get_worker_version(self, worker_version_id: str) -> dict: """ Retrieve a worker version, using the [ElementsWorker][arkindex_worker.worker.ElementsWorker]'s internal cache when possible. @@ -23,6 +40,7 @@ class WorkerVersionMixin(object): return worker_version + @worker_version_deprecation def get_worker_version_slug(self, worker_version_id: str) -> str: """ Retrieve the slug of the worker of a worker version, from a worker version UUID. diff --git a/docs/contents/implem/configure.md b/docs/contents/implem/configure.md index 59865d18c530c244812cbaa0907b5a625632c065..0e9a83731aa9eea9200baace0329ce435ae47b77 100644 --- a/docs/contents/implem/configure.md +++ b/docs/contents/implem/configure.md @@ -149,6 +149,3 @@ Many attributes are set on the worker during at the configuration stage. Here is `worker_run_id` : The ID of the `WorkerRun` corresponding object on the Arkindex instance. In Arkindex mode, this is used in `RetrieveWorkerRun` API call to retrieve the configuration and other necessary information. In developer mode, this is not set nor used. - -`worker_version_id` -: The ID of the `WorkerVersion` object linked to the current `WorkerRun`. Like the `worker_run_id` attribute, this is not set nor used in developer mode. diff --git a/tests/test_base_worker.py b/tests/test_base_worker.py index d229944a12704cbb9be075362d2c8d0f93ada125..c89df9a4d4203b96544c31a0bf87a74193e0513d 100644 --- a/tests/test_base_worker.py +++ b/tests/test_base_worker.py @@ -86,7 +86,6 @@ def test_cli_default(mocker, mock_worker_run_api): assert logger.level == logging.NOTSET assert worker.api_client assert worker.config == {"someKey": "someValue"} # from API - assert worker.worker_version_id == "12341234-1234-1234-1234-123412341234" logger.setLevel(logging.NOTSET) @@ -98,14 +97,12 @@ def test_cli_arg_verbose_given(mocker, mock_worker_run_api): mocker.patch.object(sys, "argv", ["worker", "-v"]) worker.args = worker.parser.parse_args() assert worker.is_read_only is False - assert worker.worker_run_id == "56785678-5678-5678-5678-567856785678" worker.configure() assert worker.args.verbose assert logger.level == logging.DEBUG assert worker.api_client assert worker.config == {"someKey": "someValue"} # from API - assert worker.worker_version_id == "12341234-1234-1234-1234-123412341234" logger.setLevel(logging.NOTSET) @@ -124,7 +121,6 @@ def test_cli_envvar_debug_given(mocker, monkeypatch, mock_worker_run_api): assert logger.level == logging.DEBUG assert worker.api_client assert worker.config == {"someKey": "someValue"} # from API - assert worker.worker_version_id == "12341234-1234-1234-1234-123412341234" logger.setLevel(logging.NOTSET) @@ -198,7 +194,6 @@ def test_configure_worker_run(mocker, monkeypatch, responses): worker.configure() - assert worker.worker_version_id == "12341234-1234-1234-1234-123412341234" assert worker.user_configuration == {"a": "b"} @@ -489,7 +484,6 @@ def test_configure_load_model_configuration(mocker, monkeypatch, responses): worker.configure() - assert worker.worker_version_id == "12341234-1234-1234-1234-123412341234" assert worker.model_configuration == { "param1": "value1", "param2": 2, diff --git a/tests/test_elements_worker/test_elements.py b/tests/test_elements_worker/test_elements.py index 273e41ba54a041abb435218086cd17529f58a12b..ce2b272bff8d63ff7341840dda3d7ff5d321b247 100644 --- a/tests/test_elements_worker/test_elements.py +++ b/tests/test_elements_worker/test_elements.py @@ -1524,13 +1524,11 @@ def test_list_element_children_wrong_with_metadata(mock_elements_worker): @pytest.mark.parametrize( "param, value", ( - ("worker_version", 1234), ("worker_run", 1234), - ("transcription_worker_version", 1234), ("transcription_worker_run", 1234), ), ) -def test_list_element_children_wrong_worker_version(mock_elements_worker, param, value): +def test_list_element_children_wrong_worker_run(mock_elements_worker, param, value): elt = Element({"id": "12341234-1234-1234-1234-123412341234"}) with pytest.raises(AssertionError) as e: @@ -1541,16 +1539,32 @@ def test_list_element_children_wrong_worker_version(mock_elements_worker, param, assert str(e.value) == f"{param} should be of type str or bool" +@pytest.mark.parametrize( + "param, value", + ( + ("worker_version", 1234), + ("transcription_worker_version", 1234), + ), +) +def test_list_element_children_wrong_worker_version(mock_elements_worker, param, value): + elt = Element({"id": "12341234-1234-1234-1234-123412341234"}) + + with pytest.raises(AssertionError) as e, pytest.deprecated_call(): + mock_elements_worker.list_element_children( + element=elt, + **{param: value}, + ) + assert str(e.value) == f"{param} should be of type str or bool" + + @pytest.mark.parametrize( "param", ( - ("worker_version"), ("worker_run"), - ("transcription_worker_version"), ("transcription_worker_run"), ), ) -def test_list_element_children_wrong_bool_worker_version(mock_elements_worker, param): +def test_list_element_children_wrong_bool_worker_run(mock_elements_worker, param): elt = Element({"id": "12341234-1234-1234-1234-123412341234"}) with pytest.raises(AssertionError) as e: @@ -1561,6 +1575,24 @@ def test_list_element_children_wrong_bool_worker_version(mock_elements_worker, p assert str(e.value) == f"if of type bool, {param} can only be set to False" +@pytest.mark.parametrize( + "param", + ( + ("worker_version"), + ("transcription_worker_version"), + ), +) +def test_list_element_children_wrong_bool_worker_version(mock_elements_worker, param): + elt = Element({"id": "12341234-1234-1234-1234-123412341234"}) + + with pytest.raises(AssertionError) as e, pytest.deprecated_call(): + mock_elements_worker.list_element_children( + element=elt, + **{param: True}, + ) + assert str(e.value) == f"if of type bool, {param} can only be set to False" + + def test_list_element_children_api_error(responses, mock_elements_worker): elt = Element({"id": "12341234-1234-1234-1234-123412341234"}) responses.add( @@ -1695,11 +1727,13 @@ def test_list_element_children_manual_worker_version(responses, mock_elements_wo "results": expected_children, }, ) - - for idx, child in enumerate( - mock_elements_worker.list_element_children(element=elt, worker_version=False) - ): - assert child == expected_children[idx] + with pytest.deprecated_call(): + for idx, child in enumerate( + mock_elements_worker.list_element_children( + element=elt, worker_version=False + ) + ): + assert child == expected_children[idx] assert len(responses.calls) == len(BASE_API_CALLS) + 1 assert [ @@ -1792,67 +1826,96 @@ def test_list_element_children_with_cache_unhandled_param( }, ("22222222-2222-2222-2222-222222222222",), ), - # Filter on element and worker version should give first two elements + # Filter on element and worker run should give second ( { "element": CachedElement(id="12341234-1234-1234-1234-123412341234"), - "worker_version": "56785678-5678-5678-5678-567856785678", + "worker_run": "56785678-5678-5678-5678-567856785678", }, - ( - "11111111-1111-1111-1111-111111111111", - "22222222-2222-2222-2222-222222222222", - ), + ("22222222-2222-2222-2222-222222222222",), ), - # Filter on element, type something and worker version should give first + # Filter on element, manual worker run should give first and third ( { "element": CachedElement(id="12341234-1234-1234-1234-123412341234"), - "type": "something", - "worker_version": "56785678-5678-5678-5678-567856785678", + "worker_run": False, }, - ("11111111-1111-1111-1111-111111111111",), + ( + "11111111-1111-1111-1111-111111111111", + "33333333-3333-3333-3333-333333333333", + ), ), - # Filter on element, manual worker version should give third + ), +) +def test_list_element_children_with_cache( + responses, + mock_elements_worker_with_cache, + mock_cached_elements, + filters, + expected_ids, +): + # Check we have 5 elements already present in database + assert CachedElement.select().count() == 5 + + # Query database through cache + elements = mock_elements_worker_with_cache.list_element_children(**filters) + assert elements.count() == len(expected_ids) + for child, expected_id in zip(elements.order_by("id"), expected_ids): + assert child.id == UUID(expected_id) + + # Check the worker never hits the API for elements + assert len(responses.calls) == len(BASE_API_CALLS) + assert [ + (call.request.method, call.request.url) for call in responses.calls + ] == BASE_API_CALLS + + +@pytest.mark.parametrize( + "filters, expected_ids", + ( + # Filter on element and worker version ( { "element": CachedElement(id="12341234-1234-1234-1234-123412341234"), - "worker_version": False, + "worker_version": "56785678-5678-5678-5678-567856785678", }, - ("33333333-3333-3333-3333-333333333333",), + ( + "11111111-1111-1111-1111-111111111111", + "22222222-2222-2222-2222-222222222222", + ), ), - # Filter on element and worker run should give second + # Filter on element, type double_page and worker version ( { "element": CachedElement(id="12341234-1234-1234-1234-123412341234"), - "worker_run": "56785678-5678-5678-5678-567856785678", + "type": "page", + "worker_version": "56785678-5678-5678-5678-567856785678", }, ("22222222-2222-2222-2222-222222222222",), ), - # Filter on element, manual worker run should give first and third + # Filter on element, manual worker version ( { "element": CachedElement(id="12341234-1234-1234-1234-123412341234"), - "worker_run": False, + "worker_version": False, }, - ( - "11111111-1111-1111-1111-111111111111", - "33333333-3333-3333-3333-333333333333", - ), + ("33333333-3333-3333-3333-333333333333",), ), ), ) -def test_list_element_children_with_cache( +def test_list_element_children_with_cache_deprecation( responses, mock_elements_worker_with_cache, mock_cached_elements, filters, expected_ids, ): - # Check we have 5 elements already present in database + # Check we have 2 elements already present in database assert CachedElement.select().count() == 5 - # Query database through cache - elements = mock_elements_worker_with_cache.list_element_children(**filters) + with pytest.deprecated_call(): + # Query database through cache + elements = mock_elements_worker_with_cache.list_element_children(**filters) assert elements.count() == len(expected_ids) for child, expected_id in zip(elements.order_by("id"), expected_ids): assert child.id == UUID(expected_id) @@ -2226,87 +2289,3 @@ def test_list_element_parents_with_cache_unhandled_param( str(e.value) == "When using the local cache, you can only filter by 'type' and/or 'worker_version' and/or 'worker_run'" ) - - -@pytest.mark.parametrize( - "filters, expected_id", - ( - # Filter on element - ( - { - "element": CachedElement(id="11111111-1111-1111-1111-111111111111"), - }, - "12341234-1234-1234-1234-123412341234", - ), - # Filter on element and double_page - ( - { - "element": CachedElement(id="22222222-2222-2222-2222-222222222222"), - "type": "double_page", - }, - "12341234-1234-1234-1234-123412341234", - ), - # Filter on element and worker version - ( - { - "element": CachedElement(id="33333333-3333-3333-3333-333333333333"), - "worker_version": "56785678-5678-5678-5678-567856785678", - }, - "12341234-1234-1234-1234-123412341234", - ), - # Filter on element, type double_page and worker version - ( - { - "element": CachedElement(id="11111111-1111-1111-1111-111111111111"), - "type": "double_page", - "worker_version": "56785678-5678-5678-5678-567856785678", - }, - "12341234-1234-1234-1234-123412341234", - ), - # Filter on element, manual worker version - ( - { - "element": CachedElement(id="12341234-1234-1234-1234-123412341234"), - "worker_version": False, - }, - "99999999-9999-9999-9999-999999999999", - ), - # Filter on element and worker run - ( - { - "element": CachedElement(id="22222222-2222-2222-2222-222222222222"), - "worker_run": "56785678-5678-5678-5678-567856785678", - }, - "12341234-1234-1234-1234-123412341234", - ), - # Filter on element, manual worker run - ( - { - "element": CachedElement(id="12341234-1234-1234-1234-123412341234"), - "worker_run": False, - }, - "99999999-9999-9999-9999-999999999999", - ), - ), -) -def test_list_element_parents_with_cache( - responses, - mock_elements_worker_with_cache, - mock_cached_elements, - filters, - expected_id, -): - # Check we have 5 elements already present in database - assert CachedElement.select().count() == 5 - - # Query database through cache - elements = mock_elements_worker_with_cache.list_element_parents(**filters) - assert elements.count() == 1 - for parent in elements.order_by("id"): - assert parent.id == UUID(expected_id) - - # Check the worker never hits the API for elements - assert len(responses.calls) == len(BASE_API_CALLS) - assert [ - (call.request.method, call.request.url) for call in responses.calls - ] == BASE_API_CALLS diff --git a/tests/test_elements_worker/test_worker.py b/tests/test_elements_worker/test_worker.py index 859e0fa9dcfff6c53a710a92a849f0a0198d35c7..e35e2edd9f9b299896a691bb8e47df78a33fbe2a 100644 --- a/tests/test_elements_worker/test_worker.py +++ b/tests/test_elements_worker/test_worker.py @@ -21,7 +21,8 @@ def test_get_worker_version(fake_dummy_worker): api_client.add_response("RetrieveWorkerVersion", response, id=TEST_VERSION_ID) - res = fake_dummy_worker.get_worker_version(TEST_VERSION_ID) + with pytest.deprecated_call(): + res = fake_dummy_worker.get_worker_version(TEST_VERSION_ID) assert res == response assert fake_dummy_worker._worker_version_cache[TEST_VERSION_ID] == response @@ -34,8 +35,9 @@ def test_get_worker_version__uses_cache(fake_dummy_worker): api_client.add_response("RetrieveWorkerVersion", response, id=TEST_VERSION_ID) - response_1 = fake_dummy_worker.get_worker_version(TEST_VERSION_ID) - response_2 = fake_dummy_worker.get_worker_version(TEST_VERSION_ID) + with pytest.deprecated_call(): + response_1 = fake_dummy_worker.get_worker_version(TEST_VERSION_ID) + response_2 = fake_dummy_worker.get_worker_version(TEST_VERSION_ID) assert response_1 == response assert response_1 == response_2 @@ -51,13 +53,13 @@ def test_get_worker_version_slug(mocker, fake_dummy_worker): "id": TEST_VERSION_ID, "worker": {"slug": "mock_slug"}, } - - slug = fake_dummy_worker.get_worker_version_slug(TEST_VERSION_ID) + with pytest.deprecated_call(): + slug = fake_dummy_worker.get_worker_version_slug(TEST_VERSION_ID) assert slug == "mock_slug" def test_get_worker_version_slug_none(fake_dummy_worker): - with pytest.raises(ValueError) as excinfo: + with pytest.raises(ValueError) as excinfo, pytest.deprecated_call(): fake_dummy_worker.get_worker_version_slug(None) assert str(excinfo.value) == "No worker version ID" diff --git a/tests/test_reporting.py b/tests/test_reporting.py index af214f011978d1df5c0a07b142ace8ab75493acf..6407225f1b42c7abde3ebd136516f76fdb641a4f 100644 --- a/tests/test_reporting.py +++ b/tests/test_reporting.py @@ -12,14 +12,14 @@ from arkindex_worker.reporting import Reporter def test_init(): - version_id = str(uuid.uuid4()) - reporter = Reporter(name="Worker", slug="worker-slug", version=version_id) + run_id = str(uuid.uuid4()) + reporter = Reporter(name="Worker", slug="worker-slug", run=run_id) assert "started" in reporter.report_data del reporter.report_data["started"] assert reporter.report_data == { "name": "Worker", "slug": "worker-slug", - "version": version_id, + "run": run_id, "elements": {}, } @@ -270,8 +270,8 @@ def test_reporter_save(mocker): datetime_mock = mocker.MagicMock() datetime_mock.utcnow.return_value = datetime(2000, 1, 1) mocker.patch("arkindex_worker.reporting.datetime", datetime_mock) - version_id = str(uuid.uuid4()) - reporter = Reporter(name="Worker", slug="worker-slug", version=version_id) + run_id = str(uuid.uuid4()) + reporter = Reporter(name="Worker", slug="worker-slug", run=run_id) reporter.add_element("myelement", type="text_line", type_count=4) with NamedTemporaryFile() as f: reporter.save(f.name) @@ -280,7 +280,7 @@ def test_reporter_save(mocker): "name": "Worker", "slug": "worker-slug", "started": "2000-01-01T00:00:00", - "version": version_id, + "run": run_id, "elements": { "myelement": { "classifications": {},