From f7288e9474695a44a704cc7b3e80d021a102dcbe Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 22 Apr 2024 14:51:19 +0200 Subject: [PATCH 01/69] first step --- src/dsp_tools/commands/xmlupload/xmlupload.py | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index ca7deb17b..47efe4584 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -410,20 +410,11 @@ def _upload_resources( failed_uploads.append(resource.res_id) continue - res = None try: res = _create_resource(resource, media_info, resource_create_client) - if res == (None, None): - # resource creation failed gracefully: register it as failed, then continue - failed_uploads.append(resource.res_id) - continue - else: - # resource creation succeeded: update the iri_resolver and remove the resource from the list - iri, label = res - _tidy_up_resource_creation(iri, label, iri_resolver, resource, current_res, total_res) # type: ignore[arg-type] - except PermanentTimeOutError: + except (PermanentTimeOutError, KeyboardInterrupt) as err: msg = ( - f"There was a timeout while trying to create resource '{resource.res_id}'.\n" + f"There was a {type(err).__name__} while trying to create resource '{resource.res_id}'.\n" f"It is unclear if the resource '{resource.res_id}' was created successfully or not.\n" f"Please check manually in the DSP-APP or DB.\n" f"In case of successful creation, call 'resume-xmlupload' with the flag " @@ -432,33 +423,42 @@ def _upload_resources( ) logger.error(msg) raise XmlUploadInterruptedError(msg) - except BaseException as err: - if res and res[0]: - # creation succeeded, but during tidy up, a Keyboard Interrupt occurred. tidy up again before escalating - iri, label = res - _tidy_up_resource_creation(iri, label, iri_resolver, resource, current_res, total_res) - else: - # unhandled exception during resource creation - failed_uploads.append(resource.res_id) - raise err from None - finally: - resources.remove(resource) + + try: + _tidy_up_resource_creation_idempotent( + res, resources, resource, failed_uploads, iri_resolver, current_res, total_res + ) + except KeyboardInterrupt: + _tidy_up_resource_creation_idempotent( + res, resources, resource, failed_uploads, iri_resolver, current_res, total_res + ) return iri_resolver, failed_uploads -def _tidy_up_resource_creation( - iri: str, - label: str, - iri_resolver: IriResolver, +def _tidy_up_resource_creation_idempotent( + result: tuple[str, str] | tuple[None, None], + resources: list[XMLResource], resource: XMLResource, + failed_uploads: list[str], + iri_resolver: IriResolver, current_res: int, total_res: int, ) -> None: - iri_resolver.update(resource.res_id, iri) - resource_designation = f"'{label}' (ID: '{resource.res_id}', IRI: '{iri}')" - print(f"{datetime.now()}: Created resource {current_res}/{total_res}: {resource_designation}") - logger.info(f"Created resource {current_res}/{total_res}: {resource_designation}") + iri, label = result + if iri and label: + # resource creation succeeded: update the iri_resolver + iri_resolver.update(resource.res_id, iri) + msg = f"Created resource {current_res}/{total_res}: '{label}' (ID: '{resource.res_id}', IRI: '{iri}')" + print(f"{datetime.now()}: {msg}") + logger.info(msg) + else: # noqa: PLR5501 + # resource creation failed gracefully: register it as failed + if resource.res_id not in failed_uploads: + failed_uploads.append(resource.res_id) + + if resource in resources: + resources.remove(resource) def _create_resource( From 31989449f01787023475da29e708b3c55623241c Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 22 Apr 2024 14:51:28 +0200 Subject: [PATCH 02/69] typo --- .../commands/xmlupload/models/ontology_problem_models.py | 2 +- .../commands/xmlupload/test_check_consistency_with_ontology.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/models/ontology_problem_models.py b/src/dsp_tools/commands/xmlupload/models/ontology_problem_models.py index 4714dff60..8df8f3934 100644 --- a/src/dsp_tools/commands/xmlupload/models/ontology_problem_models.py +++ b/src/dsp_tools/commands/xmlupload/models/ontology_problem_models.py @@ -139,7 +139,7 @@ def execute_problem_protocol(self) -> tuple[str, pd.DataFrame | None]: the error message and a dataframe with the errors if they exceed the maximum allowed print statements """ base_msg = ( - "\nSome text encodings used in the XML data file is not conform with the gui_element " + "\nSome text encodings used in the XML data file are not conform with the gui_element " "specified in the JSON ontology.\n" "Please consult the ontology regarding the assigned gui_elements." ) diff --git a/test/unittests/commands/xmlupload/test_check_consistency_with_ontology.py b/test/unittests/commands/xmlupload/test_check_consistency_with_ontology.py index f26400cfa..0a8c7459a 100644 --- a/test/unittests/commands/xmlupload/test_check_consistency_with_ontology.py +++ b/test/unittests/commands/xmlupload/test_check_consistency_with_ontology.py @@ -616,7 +616,7 @@ def test_analyse_all_text_value_encodings_are_correct_problems() -> None: formatted_text_props={":hasRichtext"}, unformatted_text_props={":hasSimpleText", ":hasText"} ) expected_msg = ( - "\nSome text encodings used in the XML data file is not conform with the gui_element " + "\nSome text encodings used in the XML data file are not conform with the gui_element " "specified in the JSON ontology.\n" "Please consult the ontology regarding the assigned gui_elements." "\n\n---------------------------------------\n\n" From 2fef9223e15d6b3fd05b289b3bae90557a098ade Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 22 Apr 2024 16:48:48 +0200 Subject: [PATCH 03/69] factor out _upload_one_resource (primitive approach) --- src/dsp_tools/commands/xmlupload/xmlupload.py | 94 ++++++++++++------- 1 file changed, 62 insertions(+), 32 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index 47efe4584..23f4f428c 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -399,41 +399,71 @@ def _upload_resources( # if the interrupt_after value is not set, the upload will not be interrupted interrupt_after = config.interrupt_after or total_res + 1 - for i, resource in enumerate(resources.copy()): - current_res = i + 1 + previous_total - if i >= interrupt_after: - raise XmlUploadInterruptedError(f"Interrupted: Maximum number of resources was reached ({interrupt_after})") - success, media_info = handle_media_info( - resource, config.media_previously_uploaded, sipi_server, imgdir, permissions_lookup + for resource in resources.copy(): + _upload_one_resource( + resources=resources, + resource=resource, + failed_uploads=failed_uploads, + imgdir=imgdir, + sipi_server=sipi_server, + permissions_lookup=permissions_lookup, + resource_create_client=resource_create_client, + iri_resolver=iri_resolver, + previous_total=previous_total, + total_res=total_res, + interrupt_after=interrupt_after, + config=config, ) - if not success: - failed_uploads.append(resource.res_id) - continue - - try: - res = _create_resource(resource, media_info, resource_create_client) - except (PermanentTimeOutError, KeyboardInterrupt) as err: - msg = ( - f"There was a {type(err).__name__} while trying to create resource '{resource.res_id}'.\n" - f"It is unclear if the resource '{resource.res_id}' was created successfully or not.\n" - f"Please check manually in the DSP-APP or DB.\n" - f"In case of successful creation, call 'resume-xmlupload' with the flag " - f"'--skip-first-resource' to prevent duplication.\n" - f"If not, a normal 'resume-xmlupload' can be started." - ) - logger.error(msg) - raise XmlUploadInterruptedError(msg) + return iri_resolver, failed_uploads - try: - _tidy_up_resource_creation_idempotent( - res, resources, resource, failed_uploads, iri_resolver, current_res, total_res - ) - except KeyboardInterrupt: - _tidy_up_resource_creation_idempotent( - res, resources, resource, failed_uploads, iri_resolver, current_res, total_res - ) - return iri_resolver, failed_uploads +def _upload_one_resource( + resources: list[XMLResource], + resource: XMLResource, + failed_uploads: list[str], + imgdir: str, + sipi_server: Sipi, + permissions_lookup: dict[str, Permissions], + resource_create_client: ResourceCreateClient, + iri_resolver: IriResolver, + previous_total: int, + total_res: int, + interrupt_after: int, + config: UploadConfig, +) -> None: + counter = resources.index(resource) + current_res = counter + 1 + previous_total + if counter >= interrupt_after: + raise XmlUploadInterruptedError(f"Interrupted: Maximum number of resources was reached ({interrupt_after})") + success, media_info = handle_media_info( + resource, config.media_previously_uploaded, sipi_server, imgdir, permissions_lookup + ) + if not success: + failed_uploads.append(resource.res_id) + return + + try: + res = _create_resource(resource, media_info, resource_create_client) + except (PermanentTimeOutError, KeyboardInterrupt) as err: + msg = ( + f"There was a {type(err).__name__} while trying to create resource '{resource.res_id}'.\n" + f"It is unclear if the resource '{resource.res_id}' was created successfully or not.\n" + f"Please check manually in the DSP-APP or DB.\n" + f"In case of successful creation, call 'resume-xmlupload' with the flag " + f"'--skip-first-resource' to prevent duplication.\n" + f"If not, a normal 'resume-xmlupload' can be started." + ) + logger.error(msg) + raise XmlUploadInterruptedError(msg) from None + + try: + _tidy_up_resource_creation_idempotent( + res, resources, resource, failed_uploads, iri_resolver, current_res, total_res + ) + except KeyboardInterrupt: + _tidy_up_resource_creation_idempotent( + res, resources, resource, failed_uploads, iri_resolver, current_res, total_res + ) def _tidy_up_resource_creation_idempotent( From b42fdca0e2d134523a4d5480c872225386b6fd90 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Wed, 24 Apr 2024 09:18:28 +0200 Subject: [PATCH 04/69] add test file --- test/integration/commands/xmlupload/test_resource_creation.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 test/integration/commands/xmlupload/test_resource_creation.py diff --git a/test/integration/commands/xmlupload/test_resource_creation.py b/test/integration/commands/xmlupload/test_resource_creation.py new file mode 100644 index 000000000..e69de29bb From d9b745b9bce63ac9330f344913092109fdfa1dc0 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Wed, 24 Apr 2024 11:16:29 +0200 Subject: [PATCH 05/69] create UploadState further above in the call stack --- .../commands/xmlupload/project_client.py | 4 + src/dsp_tools/commands/xmlupload/xmlupload.py | 116 +++++++----------- .../stash/test_upload_stash_with_mock.py | 37 +++--- 3 files changed, 66 insertions(+), 91 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/project_client.py b/src/dsp_tools/commands/xmlupload/project_client.py index 74854efd3..f8e70b8e3 100644 --- a/src/dsp_tools/commands/xmlupload/project_client.py +++ b/src/dsp_tools/commands/xmlupload/project_client.py @@ -19,6 +19,10 @@ class ProjectInfo: class ProjectClient(Protocol): """Interface (protocol) for project-related requests to the DSP-API.""" + con: Connection + shortcode: str + project_info: ProjectInfo | None + def get_project_iri(self) -> str: """Get the IRI of the project to which the data is being uploaded.""" diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index 23f4f428c..c00057013 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -218,15 +218,12 @@ def upload_resources( a list of resources that could not be uploaded, and the stash items that could not be reapplied. """ + upload_state = UploadState(resources, failed_uploads, iri_resolver.lookup, stash, config, permissions_lookup) try: iri_resolver, failed_uploads = _upload_resources( - resources=resources, - failed_uploads=failed_uploads, + upload_state=upload_state, imgdir=imgdir, sipi_server=sipi_server, - permissions_lookup=permissions_lookup, - con=con, - config=config, project_client=project_client, list_client=list_client, iri_resolver=iri_resolver, @@ -234,15 +231,7 @@ def upload_resources( except BaseException as err: # noqa: BLE001 (blind-except) # The forseeable errors are already handled by failed_uploads # Here we catch the unforseeable exceptions, incl. keyboard interrupt. - _handle_upload_error( - err=err, - iri_resolver=iri_resolver, - pending_resources=resources, - failed_uploads=failed_uploads, - pending_stash=stash, - config=config, - permissions_lookup=permissions_lookup, - ) + _handle_upload_error(err, upload_state) nonapplied_stash = None try: @@ -259,15 +248,7 @@ def upload_resources( except BaseException as err: # noqa: BLE001 (blind-except) # The forseeable errors are already handled by failed_uploads and nonapplied_stash. # Here we catch the unforseeable exceptions, incl. keyboard interrupt. - _handle_upload_error( - err=err, - iri_resolver=iri_resolver, - pending_resources=resources, - failed_uploads=failed_uploads, - pending_stash=stash, - config=config, - permissions_lookup=permissions_lookup, - ) + _handle_upload_error(err, upload_state) return iri_resolver, failed_uploads, nonapplied_stash @@ -343,13 +324,9 @@ def _extract_resources_from_xml(root: etree._Element, default_ontology: str) -> def _upload_resources( - resources: list[XMLResource], - failed_uploads: list[str], + upload_state: UploadState, imgdir: str, sipi_server: Sipi, - permissions_lookup: dict[str, Permissions], - con: Connection, - config: UploadConfig, project_client: ProjectClient, list_client: ListClient, iri_resolver: IriResolver, @@ -360,13 +337,9 @@ def _upload_resources( and if a permanent exception occurs, the resource is skipped. Args: - resources: list of XMLResources to upload to DSP - failed_uploads: resources that caused an error in a previous upload + upload_state: the current state of the upload imgdir: folder containing the multimedia files sipi_server: Sipi instance - permissions_lookup: maps permission strings to Permission objects - con: connection to DSP - config: the upload configuration project_client: a client for HTTP communication with the DSP-API list_client: a client for HTTP communication with the DSP-API iri_resolver: mapping from internal IDs to IRIs @@ -383,63 +356,57 @@ def _upload_resources( listnode_lookup = list_client.get_list_node_id_to_iri_lookup() resource_create_client = ResourceCreateClient( - con=con, + con=project_client.con, project_iri=project_iri, iri_resolver=iri_resolver, json_ld_context=json_ld_context, - permissions_lookup=permissions_lookup, + permissions_lookup=upload_state.permissions_lookup, listnode_lookup=listnode_lookup, - media_previously_ingested=config.media_previously_uploaded, + media_previously_ingested=upload_state.config.media_previously_uploaded, ) - total_res = len(resources) + len(iri_resolver.lookup) + total_res = len(upload_state.pending_resources) + len(iri_resolver.lookup) previous_successful = len(iri_resolver.lookup) - previous_failed = len(failed_uploads) + previous_failed = len(upload_state.failed_uploads) previous_total = previous_successful + previous_failed - # if the interrupt_after value is not set, the upload will not be interrupted - interrupt_after = config.interrupt_after or total_res + 1 - for resource in resources.copy(): + for resource in upload_state.pending_resources.copy(): _upload_one_resource( - resources=resources, + upload_state=upload_state, resource=resource, - failed_uploads=failed_uploads, imgdir=imgdir, sipi_server=sipi_server, - permissions_lookup=permissions_lookup, resource_create_client=resource_create_client, iri_resolver=iri_resolver, previous_total=previous_total, total_res=total_res, - interrupt_after=interrupt_after, - config=config, ) - return iri_resolver, failed_uploads + return iri_resolver, upload_state.failed_uploads def _upload_one_resource( - resources: list[XMLResource], + upload_state: UploadState, resource: XMLResource, - failed_uploads: list[str], imgdir: str, sipi_server: Sipi, - permissions_lookup: dict[str, Permissions], resource_create_client: ResourceCreateClient, iri_resolver: IriResolver, previous_total: int, total_res: int, - interrupt_after: int, - config: UploadConfig, ) -> None: - counter = resources.index(resource) + counter = upload_state.pending_resources.index(resource) current_res = counter + 1 + previous_total + # if the interrupt_after value is not set, the upload will not be interrupted + interrupt_after = upload_state.config.interrupt_after or total_res + 1 if counter >= interrupt_after: - raise XmlUploadInterruptedError(f"Interrupted: Maximum number of resources was reached ({interrupt_after})") + raise XmlUploadInterruptedError( + f"Interrupted: Maximum number of resources was reached ({upload_state.config.interrupt_after})" + ) success, media_info = handle_media_info( - resource, config.media_previously_uploaded, sipi_server, imgdir, permissions_lookup + resource, upload_state.config.media_previously_uploaded, sipi_server, imgdir, upload_state.permissions_lookup ) if not success: - failed_uploads.append(resource.res_id) + upload_state.failed_uploads.append(resource.res_id) return try: @@ -458,11 +425,23 @@ def _upload_one_resource( try: _tidy_up_resource_creation_idempotent( - res, resources, resource, failed_uploads, iri_resolver, current_res, total_res + res, + upload_state.pending_resources, + resource, + upload_state.failed_uploads, + iri_resolver, + current_res, + total_res, ) except KeyboardInterrupt: _tidy_up_resource_creation_idempotent( - res, resources, resource, failed_uploads, iri_resolver, current_res, total_res + res, + upload_state.pending_resources, + resource, + upload_state.failed_uploads, + iri_resolver, + current_res, + total_res, ) @@ -519,12 +498,7 @@ def _create_resource( def _handle_upload_error( err: BaseException, - iri_resolver: IriResolver, - pending_resources: list[XMLResource], - failed_uploads: list[str], - pending_stash: Stash | None, - config: UploadConfig, - permissions_lookup: dict[str, Permissions], + upload_state: UploadState, ) -> None: """ In case the xmlupload must be interrupted, @@ -538,12 +512,7 @@ def _handle_upload_error( Args: err: the error that was the cause of the abort - iri_resolver: a resolver for internal IDs to IRIs - pending_resources: resources that were not uploaded to DSP - failed_uploads: resources that caused an error when uploading to DSP - pending_stash: an object that contains all stashed links that could not yet be reapplied to their resources - config: the upload configuration - permissions_lookup: dictionary that contains the permission name as string and the corresponding Python object + upload_state: the current state of the upload """ if isinstance(err, XmlUploadInterruptedError): msg = "\n==========================================\n" + err.message + "\n" @@ -557,13 +526,10 @@ def _handle_upload_error( ) exit_code = 1 - upload_state = UploadState( - pending_resources, failed_uploads, iri_resolver.lookup, pending_stash, config, permissions_lookup - ) msg += _save_upload_state(upload_state) - if failed_uploads: - msg += f"Independently from this, there were some resources that could not be uploaded: {failed_uploads}\n" + if failed := upload_state.failed_uploads: + msg += f"Independently from this, there were some resources that could not be uploaded: {failed}\n" if exit_code == 1: logger.exception(msg) diff --git a/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py b/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py index 8dddf9893..1fe49ab3e 100644 --- a/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py +++ b/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py @@ -6,6 +6,7 @@ from dsp_tools.commands.xmlupload.iri_resolver import IriResolver from dsp_tools.commands.xmlupload.models.formatted_text_value import FormattedTextValue +from dsp_tools.commands.xmlupload.project_client import ProjectInfo from dsp_tools.commands.xmlupload.stash.stash_models import LinkValueStash from dsp_tools.commands.xmlupload.stash.stash_models import LinkValueStashItem from dsp_tools.commands.xmlupload.stash.stash_models import StandoffStash @@ -18,22 +19,6 @@ # ruff: noqa: D102 (undocumented-public-method) -class ProjectClientStub: - """Stub class for ProjectClient.""" - - def get_project_iri(self) -> str: - raise NotImplementedError("get_project_iri not implemented") - - def get_ontology_iris(self) -> list[str]: - raise NotImplementedError("get_project_iri not implemented") - - def get_ontology_name_dict(self) -> dict[str, str]: - return {} - - def get_ontology_iri_dict(self) -> dict[str, str]: - raise NotImplementedError("get_project_iri not implemented") - - @dataclass class ConnectionMock(ConnectionMockBase): """Mock class for Connection.""" @@ -68,6 +53,26 @@ def put( return self.put_responses.pop(0) +class ProjectClientStub: + """Stub class for ProjectClient.""" + + con: Connection = ConnectionMock(post_responses=[{}]) + shortcode: str = "1234" + project_info: ProjectInfo | None = None + + def get_project_iri(self) -> str: + raise NotImplementedError("get_project_iri not implemented") + + def get_ontology_iris(self) -> list[str]: + raise NotImplementedError("get_project_iri not implemented") + + def get_ontology_name_dict(self) -> dict[str, str]: + return {} + + def get_ontology_iri_dict(self) -> dict[str, str]: + raise NotImplementedError("get_project_iri not implemented") + + class TestUploadLinkValueStashes: def test_upload_link_value_stash(self) -> None: """Upload stashed link values (resptr), if all goes well.""" From 9ea7f46e088d28a2a7c3a78299e1d98d7b004a1a Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Wed, 24 Apr 2024 11:50:41 +0200 Subject: [PATCH 06/69] beautify --- src/dsp_tools/commands/xmlupload/xmlupload.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index c00057013..4687a80f2 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -365,7 +365,6 @@ def _upload_resources( media_previously_ingested=upload_state.config.media_previously_uploaded, ) - total_res = len(upload_state.pending_resources) + len(iri_resolver.lookup) previous_successful = len(iri_resolver.lookup) previous_failed = len(upload_state.failed_uploads) previous_total = previous_successful + previous_failed @@ -379,7 +378,6 @@ def _upload_resources( resource_create_client=resource_create_client, iri_resolver=iri_resolver, previous_total=previous_total, - total_res=total_res, ) return iri_resolver, upload_state.failed_uploads @@ -392,8 +390,8 @@ def _upload_one_resource( resource_create_client: ResourceCreateClient, iri_resolver: IriResolver, previous_total: int, - total_res: int, ) -> None: + total_res = len(upload_state.pending_resources) + len(iri_resolver.lookup) counter = upload_state.pending_resources.index(resource) current_res = counter + 1 + previous_total # if the interrupt_after value is not set, the upload will not be interrupted @@ -402,6 +400,7 @@ def _upload_one_resource( raise XmlUploadInterruptedError( f"Interrupted: Maximum number of resources was reached ({upload_state.config.interrupt_after})" ) + success, media_info = handle_media_info( resource, upload_state.config.media_previously_uploaded, sipi_server, imgdir, upload_state.permissions_lookup ) @@ -410,7 +409,7 @@ def _upload_one_resource( return try: - res = _create_resource(resource, media_info, resource_create_client) + result = _create_resource(resource, media_info, resource_create_client) except (PermanentTimeOutError, KeyboardInterrupt) as err: msg = ( f"There was a {type(err).__name__} while trying to create resource '{resource.res_id}'.\n" @@ -425,7 +424,7 @@ def _upload_one_resource( try: _tidy_up_resource_creation_idempotent( - res, + result, upload_state.pending_resources, resource, upload_state.failed_uploads, @@ -435,7 +434,7 @@ def _upload_one_resource( ) except KeyboardInterrupt: _tidy_up_resource_creation_idempotent( - res, + result, upload_state.pending_resources, resource, upload_state.failed_uploads, @@ -496,10 +495,7 @@ def _create_resource( return None, None -def _handle_upload_error( - err: BaseException, - upload_state: UploadState, -) -> None: +def _handle_upload_error(err: BaseException, upload_state: UploadState) -> None: """ In case the xmlupload must be interrupted, e.g. because of an error that could not be handled, From fb484929880ff6b1c87ccc830d2456f87ed7a340 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Wed, 24 Apr 2024 14:33:41 +0200 Subject: [PATCH 07/69] edit --- src/dsp_tools/commands/xmlupload/xmlupload.py | 72 +++++++------------ 1 file changed, 26 insertions(+), 46 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index 4687a80f2..564636bd4 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -106,16 +106,14 @@ def xmlupload( project_client: ProjectClient = ProjectClientLive(con, config.shortcode) list_client: ListClient = ListClientLive(con, project_client.get_project_iri()) iri_resolver = IriResolver() + upload_state = UploadState(resources, [], iri_resolver.lookup, stash, config, permissions_lookup) iri_resolver, failed_uploads, nonapplied_stash = upload_resources( - resources=resources, - failed_uploads=[], + upload_state=upload_state, imgdir=imgdir, sipi_server=sipi_server, - permissions_lookup=permissions_lookup, con=con, stash=stash, - config=config, project_client=project_client, list_client=list_client, iri_resolver=iri_resolver, @@ -185,14 +183,11 @@ def _prepare_upload( def upload_resources( - resources: list[XMLResource], - failed_uploads: list[str], + upload_state: UploadState, imgdir: str, sipi_server: Sipi, - permissions_lookup: dict[str, Permissions], con: Connection, stash: Stash | None, - config: UploadConfig, project_client: ProjectClient, list_client: ListClient, iri_resolver: IriResolver, @@ -201,14 +196,11 @@ def upload_resources( Actual upload of all resources to DSP. Args: - resources: list of XMLResources to upload to DSP - failed_uploads: resources that caused an error in a previous upload + upload_state: the current state of the upload imgdir: folder containing the multimedia files sipi_server: Sipi instance - permissions_lookup: dictionary that contains the permission name as string and the corresponding Python object con: connection to the DSP server stash: an object that contains all stashed links that could not be reapplied to their resources - config: the upload configuration project_client: a client for HTTP communication with the DSP-API list_client: a client for HTTP communication with the DSP-API iri_resolver: mapping from internal IDs to IRIs @@ -218,7 +210,6 @@ def upload_resources( a list of resources that could not be uploaded, and the stash items that could not be reapplied. """ - upload_state = UploadState(resources, failed_uploads, iri_resolver.lookup, stash, config, permissions_lookup) try: iri_resolver, failed_uploads = _upload_resources( upload_state=upload_state, @@ -391,16 +382,7 @@ def _upload_one_resource( iri_resolver: IriResolver, previous_total: int, ) -> None: - total_res = len(upload_state.pending_resources) + len(iri_resolver.lookup) - counter = upload_state.pending_resources.index(resource) - current_res = counter + 1 + previous_total - # if the interrupt_after value is not set, the upload will not be interrupted - interrupt_after = upload_state.config.interrupt_after or total_res + 1 - if counter >= interrupt_after: - raise XmlUploadInterruptedError( - f"Interrupted: Maximum number of resources was reached ({upload_state.config.interrupt_after})" - ) - + current_res, total_res = _compute_counter_info_and_interrupt(upload_state, previous_total, resource) success, media_info = handle_media_info( resource, upload_state.config.media_previously_uploaded, sipi_server, imgdir, upload_state.permissions_lookup ) @@ -423,32 +405,30 @@ def _upload_one_resource( raise XmlUploadInterruptedError(msg) from None try: - _tidy_up_resource_creation_idempotent( - result, - upload_state.pending_resources, - resource, - upload_state.failed_uploads, - iri_resolver, - current_res, - total_res, - ) + _tidy_up_resource_creation_idempotent(upload_state, result, resource, iri_resolver, current_res, total_res) except KeyboardInterrupt: - _tidy_up_resource_creation_idempotent( - result, - upload_state.pending_resources, - resource, - upload_state.failed_uploads, - iri_resolver, - current_res, - total_res, + _tidy_up_resource_creation_idempotent(upload_state, result, resource, iri_resolver, current_res, total_res) + + +def _compute_counter_info_and_interrupt( + upload_state: UploadState, previous_total: int, resource: XMLResource +) -> tuple[int, int]: + total_res = len(upload_state.pending_resources) + len(upload_state.iri_resolver_lookup) + counter = upload_state.pending_resources.index(resource) + current_res = counter + 1 + previous_total + # if the interrupt_after value is not set, the upload will not be interrupted + interrupt_after = upload_state.config.interrupt_after or total_res + 1 + if counter >= interrupt_after: + raise XmlUploadInterruptedError( + f"Interrupted: Maximum number of resources was reached ({upload_state.config.interrupt_after})" ) + return current_res, total_res def _tidy_up_resource_creation_idempotent( + upload_state: UploadState, result: tuple[str, str] | tuple[None, None], - resources: list[XMLResource], resource: XMLResource, - failed_uploads: list[str], iri_resolver: IriResolver, current_res: int, total_res: int, @@ -462,11 +442,11 @@ def _tidy_up_resource_creation_idempotent( logger.info(msg) else: # noqa: PLR5501 # resource creation failed gracefully: register it as failed - if resource.res_id not in failed_uploads: - failed_uploads.append(resource.res_id) + if resource.res_id not in upload_state.failed_uploads: + upload_state.failed_uploads.append(resource.res_id) - if resource in resources: - resources.remove(resource) + if resource in upload_state.pending_resources: + upload_state.pending_resources.remove(resource) def _create_resource( From 08a0f2d2e01064b2d3ac3d570f84a0c0a9768e70 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Wed, 24 Apr 2024 14:52:07 +0200 Subject: [PATCH 08/69] fix --- .../resume_xmlupload/resume_xmlupload.py | 9 +----- src/dsp_tools/commands/xmlupload/xmlupload.py | 30 +++++-------------- .../stash/test_upload_stash_with_mock.py | 17 +++++------ 3 files changed, 16 insertions(+), 40 deletions(-) diff --git a/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py b/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py index 6b5f008f4..0f252e455 100644 --- a/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py +++ b/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py @@ -6,7 +6,6 @@ from loguru import logger from termcolor import colored -from dsp_tools.commands.xmlupload.iri_resolver import IriResolver from dsp_tools.commands.xmlupload.list_client import ListClient from dsp_tools.commands.xmlupload.list_client import ListClientLive from dsp_tools.commands.xmlupload.models.sipi import Sipi @@ -73,17 +72,11 @@ def resume_xmlupload(server: str, user: str, password: str, sipi: str, skip_firs list_client: ListClient = ListClientLive(con, project_client.get_project_iri()) iri_resolver, failed_uploads, nonapplied_stash = upload_resources( - resources=upload_state.pending_resources, - failed_uploads=upload_state.failed_uploads, + upload_state=upload_state, imgdir=".", sipi_server=sipi_server, - permissions_lookup=upload_state.permissions_lookup, - con=con, - stash=upload_state.pending_stash, - config=upload_state.config, project_client=project_client, list_client=list_client, - iri_resolver=IriResolver(upload_state.iri_resolver_lookup), ) return cleanup_upload(iri_resolver, upload_state.config, failed_uploads, nonapplied_stash) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index 564636bd4..4a8268b61 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -112,11 +112,8 @@ def xmlupload( upload_state=upload_state, imgdir=imgdir, sipi_server=sipi_server, - con=con, - stash=stash, project_client=project_client, list_client=list_client, - iri_resolver=iri_resolver, ) return cleanup_upload(iri_resolver, config, failed_uploads, nonapplied_stash) @@ -186,11 +183,8 @@ def upload_resources( upload_state: UploadState, imgdir: str, sipi_server: Sipi, - con: Connection, - stash: Stash | None, project_client: ProjectClient, list_client: ListClient, - iri_resolver: IriResolver, ) -> tuple[IriResolver, list[str], Stash | None]: """ Actual upload of all resources to DSP. @@ -199,11 +193,8 @@ def upload_resources( upload_state: the current state of the upload imgdir: folder containing the multimedia files sipi_server: Sipi instance - con: connection to the DSP server - stash: an object that contains all stashed links that could not be reapplied to their resources project_client: a client for HTTP communication with the DSP-API list_client: a client for HTTP communication with the DSP-API - iri_resolver: mapping from internal IDs to IRIs Returns: the id2iri mapping of the uploaded resources, @@ -217,7 +208,6 @@ def upload_resources( sipi_server=sipi_server, project_client=project_client, list_client=list_client, - iri_resolver=iri_resolver, ) except BaseException as err: # noqa: BLE001 (blind-except) # The forseeable errors are already handled by failed_uploads @@ -228,12 +218,11 @@ def upload_resources( try: nonapplied_stash = ( _upload_stash( - stash=stash, + stash=upload_state.pending_stash, iri_resolver=iri_resolver, - con=con, project_client=project_client, ) - if stash + if upload_state.pending_stash else None ) except BaseException as err: # noqa: BLE001 (blind-except) @@ -258,13 +247,12 @@ def _get_data_from_xml( def _upload_stash( stash: Stash, iri_resolver: IriResolver, - con: Connection, project_client: ProjectClient, ) -> Stash | None: if stash.standoff_stash: nonapplied_standoff = upload_stashed_xml_texts( iri_resolver=iri_resolver, - con=con, + con=project_client.con, stashed_xml_texts=stash.standoff_stash, ) else: @@ -273,7 +261,7 @@ def _upload_stash( if stash.link_value_stash: nonapplied_resptr_props = upload_stashed_resptr_props( iri_resolver=iri_resolver, - con=con, + con=project_client.con, stashed_resptr_props=stash.link_value_stash, context=context, ) @@ -320,7 +308,6 @@ def _upload_resources( sipi_server: Sipi, project_client: ProjectClient, list_client: ListClient, - iri_resolver: IriResolver, ) -> tuple[IriResolver, list[str]]: """ Iterates through all resources and tries to upload them to DSP. @@ -333,7 +320,6 @@ def _upload_resources( sipi_server: Sipi instance project_client: a client for HTTP communication with the DSP-API list_client: a client for HTTP communication with the DSP-API - iri_resolver: mapping from internal IDs to IRIs Raises: BaseException: in case of an unhandled exception during resource creation @@ -349,14 +335,14 @@ def _upload_resources( resource_create_client = ResourceCreateClient( con=project_client.con, project_iri=project_iri, - iri_resolver=iri_resolver, + iri_resolver=IriResolver(upload_state.iri_resolver_lookup), json_ld_context=json_ld_context, permissions_lookup=upload_state.permissions_lookup, listnode_lookup=listnode_lookup, media_previously_ingested=upload_state.config.media_previously_uploaded, ) - previous_successful = len(iri_resolver.lookup) + previous_successful = len(upload_state.iri_resolver_lookup) previous_failed = len(upload_state.failed_uploads) previous_total = previous_successful + previous_failed @@ -367,10 +353,10 @@ def _upload_resources( imgdir=imgdir, sipi_server=sipi_server, resource_create_client=resource_create_client, - iri_resolver=iri_resolver, + iri_resolver=IriResolver(upload_state.iri_resolver_lookup), previous_total=previous_total, ) - return iri_resolver, upload_state.failed_uploads + return IriResolver(upload_state.iri_resolver_lookup), upload_state.failed_uploads def _upload_one_resource( diff --git a/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py b/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py index 1fe49ab3e..bb0534264 100644 --- a/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py +++ b/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py @@ -53,12 +53,13 @@ def put( return self.put_responses.pop(0) +@dataclass class ProjectClientStub: """Stub class for ProjectClient.""" - con: Connection = ConnectionMock(post_responses=[{}]) - shortcode: str = "1234" - project_info: ProjectInfo | None = None + con: Connection + shortcode: str + project_info: ProjectInfo | None def get_project_iri(self) -> str: raise NotImplementedError("get_project_iri not implemented") @@ -91,12 +92,10 @@ def test_upload_link_value_stash(self) -> None: "002": "http://www.rdfh.ch/0001/002", } ) - con: Connection = ConnectionMock(post_responses=[{}]) nonapplied = _upload_stash( stash=stash, iri_resolver=iri_resolver, - con=con, - project_client=ProjectClientStub(), + project_client=ProjectClientStub(ConnectionMock(post_responses=[{}]), "1234", None), ) assert nonapplied is None @@ -144,8 +143,7 @@ def test_upload_text_value_stash(self) -> None: nonapplied = _upload_stash( stash=stash, iri_resolver=iri_resolver, - con=con, - project_client=ProjectClientStub(), + project_client=ProjectClientStub(con, "1234", None), ) assert nonapplied is None @@ -190,7 +188,6 @@ def test_not_upload_text_value_stash_if_uuid_not_on_value(self) -> None: nonapplied = _upload_stash( stash=stash, iri_resolver=iri_resolver, - con=con, - project_client=ProjectClientStub(), + project_client=ProjectClientStub(con, "1234", None), ) assert nonapplied == stash From a71dba5d6bb8273a1e0663ccd497c1854bd5ca40 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Wed, 24 Apr 2024 16:06:22 +0200 Subject: [PATCH 09/69] UploadState contains IriResolver instead of dict --- .../resume_xmlupload/resume_xmlupload.py | 6 ++-- .../commands/xmlupload/models/upload_state.py | 5 +-- src/dsp_tools/commands/xmlupload/xmlupload.py | 36 ++++++++----------- .../commands/xmlupload/test_upload_state.py | 5 +-- 4 files changed, 23 insertions(+), 29 deletions(-) diff --git a/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py b/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py index 0f252e455..756464338 100644 --- a/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py +++ b/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py @@ -51,7 +51,7 @@ def resume_xmlupload(server: str, user: str, password: str, sipi: str, skip_firs if resp == "n": sys.exit(1) - previous_successful = len(upload_state.iri_resolver_lookup) + previous_successful = len(upload_state.iri_resolver.lookup) previous_failed = len(upload_state.failed_uploads) previous_total = previous_successful + previous_failed msg = ( @@ -71,7 +71,7 @@ def resume_xmlupload(server: str, user: str, password: str, sipi: str, skip_firs project_client: ProjectClient = ProjectClientLive(con, upload_state.config.shortcode) list_client: ListClient = ListClientLive(con, project_client.get_project_iri()) - iri_resolver, failed_uploads, nonapplied_stash = upload_resources( + nonapplied_stash = upload_resources( upload_state=upload_state, imgdir=".", sipi_server=sipi_server, @@ -79,7 +79,7 @@ def resume_xmlupload(server: str, user: str, password: str, sipi: str, skip_firs list_client=list_client, ) - return cleanup_upload(iri_resolver, upload_state.config, failed_uploads, nonapplied_stash) + return cleanup_upload(upload_state.iri_resolver, upload_state.config, upload_state.failed_uploads, nonapplied_stash) def _read_upload_state_from_disk(server: str) -> UploadState: diff --git a/src/dsp_tools/commands/xmlupload/models/upload_state.py b/src/dsp_tools/commands/xmlupload/models/upload_state.py index 73b212c0c..a4c6b6b54 100644 --- a/src/dsp_tools/commands/xmlupload/models/upload_state.py +++ b/src/dsp_tools/commands/xmlupload/models/upload_state.py @@ -1,12 +1,13 @@ from dataclasses import dataclass +from dsp_tools.commands.xmlupload.iri_resolver import IriResolver from dsp_tools.commands.xmlupload.models.permission import Permissions from dsp_tools.commands.xmlupload.models.xmlresource import XMLResource from dsp_tools.commands.xmlupload.stash.stash_models import Stash from dsp_tools.commands.xmlupload.upload_config import UploadConfig -@dataclass(frozen=True) +@dataclass class UploadState: """ Save the state of an xmlupload, so that after an interruption, it can be resumed. @@ -14,7 +15,7 @@ class UploadState: pending_resources: list[XMLResource] failed_uploads: list[str] - iri_resolver_lookup: dict[str, str] + iri_resolver: IriResolver pending_stash: Stash | None config: UploadConfig permissions_lookup: dict[str, Permissions] diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index 4a8268b61..b631037f4 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -105,10 +105,9 @@ def xmlupload( project_client: ProjectClient = ProjectClientLive(con, config.shortcode) list_client: ListClient = ListClientLive(con, project_client.get_project_iri()) - iri_resolver = IriResolver() - upload_state = UploadState(resources, [], iri_resolver.lookup, stash, config, permissions_lookup) + upload_state = UploadState(resources, [], IriResolver(), stash, config, permissions_lookup) - iri_resolver, failed_uploads, nonapplied_stash = upload_resources( + nonapplied_stash = upload_resources( upload_state=upload_state, imgdir=imgdir, sipi_server=sipi_server, @@ -116,7 +115,7 @@ def xmlupload( list_client=list_client, ) - return cleanup_upload(iri_resolver, config, failed_uploads, nonapplied_stash) + return cleanup_upload(upload_state.iri_resolver, config, upload_state.failed_uploads, nonapplied_stash) def cleanup_upload( @@ -185,7 +184,7 @@ def upload_resources( sipi_server: Sipi, project_client: ProjectClient, list_client: ListClient, -) -> tuple[IriResolver, list[str], Stash | None]: +) -> Stash | None: """ Actual upload of all resources to DSP. @@ -202,7 +201,7 @@ def upload_resources( and the stash items that could not be reapplied. """ try: - iri_resolver, failed_uploads = _upload_resources( + _upload_resources( upload_state=upload_state, imgdir=imgdir, sipi_server=sipi_server, @@ -219,7 +218,7 @@ def upload_resources( nonapplied_stash = ( _upload_stash( stash=upload_state.pending_stash, - iri_resolver=iri_resolver, + iri_resolver=upload_state.iri_resolver, project_client=project_client, ) if upload_state.pending_stash @@ -229,7 +228,7 @@ def upload_resources( # The forseeable errors are already handled by failed_uploads and nonapplied_stash. # Here we catch the unforseeable exceptions, incl. keyboard interrupt. _handle_upload_error(err, upload_state) - return iri_resolver, failed_uploads, nonapplied_stash + return nonapplied_stash def _get_data_from_xml( @@ -308,7 +307,7 @@ def _upload_resources( sipi_server: Sipi, project_client: ProjectClient, list_client: ListClient, -) -> tuple[IriResolver, list[str]]: +) -> None: """ Iterates through all resources and tries to upload them to DSP. If a temporary exception occurs, the action is repeated until success, @@ -324,9 +323,6 @@ def _upload_resources( Raises: BaseException: in case of an unhandled exception during resource creation XmlUploadInterruptedError: if the number of resources created is equal to the interrupt_after value - - Returns: - id2iri_mapping, failed_uploads """ project_iri = project_client.get_project_iri() json_ld_context = get_json_ld_context_for_project(project_client.get_ontology_name_dict()) @@ -335,14 +331,14 @@ def _upload_resources( resource_create_client = ResourceCreateClient( con=project_client.con, project_iri=project_iri, - iri_resolver=IriResolver(upload_state.iri_resolver_lookup), + iri_resolver=upload_state.iri_resolver, json_ld_context=json_ld_context, permissions_lookup=upload_state.permissions_lookup, listnode_lookup=listnode_lookup, media_previously_ingested=upload_state.config.media_previously_uploaded, ) - previous_successful = len(upload_state.iri_resolver_lookup) + previous_successful = len(upload_state.iri_resolver.lookup) previous_failed = len(upload_state.failed_uploads) previous_total = previous_successful + previous_failed @@ -353,10 +349,8 @@ def _upload_resources( imgdir=imgdir, sipi_server=sipi_server, resource_create_client=resource_create_client, - iri_resolver=IriResolver(upload_state.iri_resolver_lookup), previous_total=previous_total, ) - return IriResolver(upload_state.iri_resolver_lookup), upload_state.failed_uploads def _upload_one_resource( @@ -365,7 +359,6 @@ def _upload_one_resource( imgdir: str, sipi_server: Sipi, resource_create_client: ResourceCreateClient, - iri_resolver: IriResolver, previous_total: int, ) -> None: current_res, total_res = _compute_counter_info_and_interrupt(upload_state, previous_total, resource) @@ -391,15 +384,15 @@ def _upload_one_resource( raise XmlUploadInterruptedError(msg) from None try: - _tidy_up_resource_creation_idempotent(upload_state, result, resource, iri_resolver, current_res, total_res) + _tidy_up_resource_creation_idempotent(upload_state, result, resource, current_res, total_res) except KeyboardInterrupt: - _tidy_up_resource_creation_idempotent(upload_state, result, resource, iri_resolver, current_res, total_res) + _tidy_up_resource_creation_idempotent(upload_state, result, resource, current_res, total_res) def _compute_counter_info_and_interrupt( upload_state: UploadState, previous_total: int, resource: XMLResource ) -> tuple[int, int]: - total_res = len(upload_state.pending_resources) + len(upload_state.iri_resolver_lookup) + total_res = len(upload_state.pending_resources) + len(upload_state.iri_resolver.lookup) counter = upload_state.pending_resources.index(resource) current_res = counter + 1 + previous_total # if the interrupt_after value is not set, the upload will not be interrupted @@ -415,14 +408,13 @@ def _tidy_up_resource_creation_idempotent( upload_state: UploadState, result: tuple[str, str] | tuple[None, None], resource: XMLResource, - iri_resolver: IriResolver, current_res: int, total_res: int, ) -> None: iri, label = result if iri and label: # resource creation succeeded: update the iri_resolver - iri_resolver.update(resource.res_id, iri) + upload_state.iri_resolver.lookup[resource.res_id] = iri msg = f"Created resource {current_res}/{total_res}: '{label}' (ID: '{resource.res_id}', IRI: '{iri}')" print(f"{datetime.now()}: {msg}") logger.info(msg) diff --git a/test/integration/commands/xmlupload/test_upload_state.py b/test/integration/commands/xmlupload/test_upload_state.py index 50af8df50..a9d4055c0 100644 --- a/test/integration/commands/xmlupload/test_upload_state.py +++ b/test/integration/commands/xmlupload/test_upload_state.py @@ -3,6 +3,7 @@ from lxml import etree +from dsp_tools.commands.xmlupload.iri_resolver import IriResolver from dsp_tools.commands.xmlupload.models.upload_state import UploadState from dsp_tools.commands.xmlupload.models.xmlresource import XMLResource from dsp_tools.commands.xmlupload.upload_config import DiagnosticsConfig @@ -23,7 +24,7 @@ def test_save_upload_state(tmp_path: Path) -> None: upload_state = UploadState( pending_resources=[XMLResource(etree.fromstring(resource_str), default_ontology="test")], failed_uploads=[], - iri_resolver_lookup={"foo": "bar"}, + iri_resolver=IriResolver({"foo": "bar"}), pending_stash=None, config=config, permissions_lookup={}, @@ -34,7 +35,7 @@ def test_save_upload_state(tmp_path: Path) -> None: assert msg == f"Saved the current upload state to {save_location}.\n" assert len(upload_state.pending_resources) == len(saved_state.pending_resources) assert [r.res_id for r in upload_state.pending_resources] == [r.res_id for r in saved_state.pending_resources] - assert upload_state.iri_resolver_lookup == saved_state.iri_resolver_lookup + assert upload_state.iri_resolver.lookup == saved_state.iri_resolver.lookup assert upload_state.pending_stash == saved_state.pending_stash assert upload_state.config == saved_state.config assert upload_state.permissions_lookup == saved_state.permissions_lookup From 0886017a5465db2de95a78e99095b70e8fbcd5e0 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Wed, 24 Apr 2024 16:28:10 +0200 Subject: [PATCH 10/69] simplify --- src/dsp_tools/commands/xmlupload/xmlupload.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index b631037f4..39457027c 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -213,16 +213,13 @@ def upload_resources( # Here we catch the unforseeable exceptions, incl. keyboard interrupt. _handle_upload_error(err, upload_state) - nonapplied_stash = None + if not upload_state.pending_stash: + return None try: - nonapplied_stash = ( - _upload_stash( - stash=upload_state.pending_stash, - iri_resolver=upload_state.iri_resolver, - project_client=project_client, - ) - if upload_state.pending_stash - else None + nonapplied_stash = _upload_stash( + stash=upload_state.pending_stash, + iri_resolver=upload_state.iri_resolver, + project_client=project_client, ) except BaseException as err: # noqa: BLE001 (blind-except) # The forseeable errors are already handled by failed_uploads and nonapplied_stash. From a2dac8a2f4dd95ed26de23c8cc31170147d50b16 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Wed, 24 Apr 2024 16:42:56 +0200 Subject: [PATCH 11/69] revert unnecessary changes in test_upload_stash_with_mock --- .../stash/test_upload_stash_with_mock.py | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py b/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py index bb0534264..85424705f 100644 --- a/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py +++ b/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py @@ -19,6 +19,27 @@ # ruff: noqa: D102 (undocumented-public-method) +@dataclass +class ProjectClientStub: + """Stub class for ProjectClient.""" + + con: Connection + shortcode: str + project_info: ProjectInfo | None + + def get_project_iri(self) -> str: + raise NotImplementedError("get_project_iri not implemented") + + def get_ontology_iris(self) -> list[str]: + raise NotImplementedError("get_project_iri not implemented") + + def get_ontology_name_dict(self) -> dict[str, str]: + return {} + + def get_ontology_iri_dict(self) -> dict[str, str]: + raise NotImplementedError("get_project_iri not implemented") + + @dataclass class ConnectionMock(ConnectionMockBase): """Mock class for Connection.""" @@ -53,27 +74,6 @@ def put( return self.put_responses.pop(0) -@dataclass -class ProjectClientStub: - """Stub class for ProjectClient.""" - - con: Connection - shortcode: str - project_info: ProjectInfo | None - - def get_project_iri(self) -> str: - raise NotImplementedError("get_project_iri not implemented") - - def get_ontology_iris(self) -> list[str]: - raise NotImplementedError("get_project_iri not implemented") - - def get_ontology_name_dict(self) -> dict[str, str]: - return {} - - def get_ontology_iri_dict(self) -> dict[str, str]: - raise NotImplementedError("get_project_iri not implemented") - - class TestUploadLinkValueStashes: def test_upload_link_value_stash(self) -> None: """Upload stashed link values (resptr), if all goes well.""" From 2b5ef2730e8f5845adcd5165c48eb5ac300adc19 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Wed, 24 Apr 2024 16:47:22 +0200 Subject: [PATCH 12/69] simplify diff --- .../commands/xmlupload/stash/test_upload_stash_with_mock.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py b/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py index 85424705f..a5b7512b0 100644 --- a/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py +++ b/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py @@ -92,10 +92,11 @@ def test_upload_link_value_stash(self) -> None: "002": "http://www.rdfh.ch/0001/002", } ) + con: Connection = ConnectionMock(post_responses=[{}]) nonapplied = _upload_stash( stash=stash, iri_resolver=iri_resolver, - project_client=ProjectClientStub(ConnectionMock(post_responses=[{}]), "1234", None), + project_client=ProjectClientStub(con, "1234", None), ) assert nonapplied is None From 2bb9ae31577a1ed6b546b9a29b12219b60106dc2 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Wed, 24 Apr 2024 17:32:06 +0200 Subject: [PATCH 13/69] one single try-except --- src/dsp_tools/commands/xmlupload/xmlupload.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index 39457027c..44f52b6d1 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -208,24 +208,18 @@ def upload_resources( project_client=project_client, list_client=list_client, ) - except BaseException as err: # noqa: BLE001 (blind-except) - # The forseeable errors are already handled by failed_uploads - # Here we catch the unforseeable exceptions, incl. keyboard interrupt. - _handle_upload_error(err, upload_state) - - if not upload_state.pending_stash: - return None - try: - nonapplied_stash = _upload_stash( + if not upload_state.pending_stash: + return None + return _upload_stash( stash=upload_state.pending_stash, iri_resolver=upload_state.iri_resolver, project_client=project_client, ) except BaseException as err: # noqa: BLE001 (blind-except) - # The forseeable errors are already handled by failed_uploads and nonapplied_stash. + # The forseeable errors are already handled by failed_uploads # Here we catch the unforseeable exceptions, incl. keyboard interrupt. _handle_upload_error(err, upload_state) - return nonapplied_stash + return None def _get_data_from_xml( From d6fe6250714d97357d63ee492e7261865957fc15 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Wed, 24 Apr 2024 17:33:46 +0200 Subject: [PATCH 14/69] fix docstring --- src/dsp_tools/commands/xmlupload/xmlupload.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index 44f52b6d1..f6b44bb7c 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -196,9 +196,7 @@ def upload_resources( list_client: a client for HTTP communication with the DSP-API Returns: - the id2iri mapping of the uploaded resources, - a list of resources that could not be uploaded, - and the stash items that could not be reapplied. + the stash items that could not be reapplied. """ try: _upload_resources( From d4b4f9bfe1671b5b0a915a1dfac29ec73b5f477d Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Wed, 24 Apr 2024 23:02:09 +0200 Subject: [PATCH 15/69] first test (not working yet) --- .../xmlupload/test_resource_creation.py | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/test/integration/commands/xmlupload/test_resource_creation.py b/test/integration/commands/xmlupload/test_resource_creation.py index e69de29bb..49ddf3488 100644 --- a/test/integration/commands/xmlupload/test_resource_creation.py +++ b/test/integration/commands/xmlupload/test_resource_creation.py @@ -0,0 +1,59 @@ +from dataclasses import dataclass +from test.integration.commands.xmlupload.connection_mock import ConnectionMockBase + +from lxml import etree + +from dsp_tools.commands.xmlupload.iri_resolver import IriResolver +from dsp_tools.commands.xmlupload.models.sipi import Sipi +from dsp_tools.commands.xmlupload.models.upload_state import UploadState +from dsp_tools.commands.xmlupload.models.xmlresource import XMLResource +from dsp_tools.commands.xmlupload.project_client import ProjectInfo +from dsp_tools.commands.xmlupload.upload_config import UploadConfig +from dsp_tools.commands.xmlupload.xmlupload import _upload_resources +from dsp_tools.utils.connection import Connection + + +class ListClientMock: + def get_list_node_id_to_iri_lookup(self) -> dict[str, str]: + return {} + + +@dataclass +class ProjectClientStub: + """Stub class for ProjectClient.""" + + con: Connection + shortcode: str + project_info: ProjectInfo | None + + def get_project_iri(self) -> str: + return "https://admin.test.dasch.swiss/project/MsOaiQkcQ7-QPxsYBKckfQ" + + def get_ontology_iris(self) -> list[str]: + raise NotImplementedError("get_project_iri not implemented") + + def get_ontology_name_dict(self) -> dict[str, str]: + return {} + + def get_ontology_iri_dict(self) -> dict[str, str]: + raise NotImplementedError("get_project_iri not implemented") + + +class ConnectionMock(ConnectionMockBase): ... + + +def test_happy_path() -> None: + xml_str = """ + + + foo_1 text + + + """ + xml_res = XMLResource(etree.fromstring(xml_str), "default_onto") + upload_state = UploadState([xml_res], [], IriResolver(), None, UploadConfig(), {}) + project_client = ProjectClientStub(ConnectionMock(), "1234", None) + _upload_resources(upload_state, ".", Sipi(ConnectionMock()), project_client, ListClientMock()) + assert not upload_state.pending_resources + assert not upload_state.failed_uploads + assert upload_state.iri_resolver.lookup From f945307c86019fab33f812db6f0ee95f08abfbe5 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Thu, 25 Apr 2024 09:11:48 +0200 Subject: [PATCH 16/69] simplify code --- src/dsp_tools/commands/xmlupload/xmlupload.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index f6b44bb7c..c89255cf2 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -359,7 +359,7 @@ def _upload_one_resource( return try: - result = _create_resource(resource, media_info, resource_create_client) + iri, label = _create_resource(resource, media_info, resource_create_client) except (PermanentTimeOutError, KeyboardInterrupt) as err: msg = ( f"There was a {type(err).__name__} while trying to create resource '{resource.res_id}'.\n" @@ -373,9 +373,9 @@ def _upload_one_resource( raise XmlUploadInterruptedError(msg) from None try: - _tidy_up_resource_creation_idempotent(upload_state, result, resource, current_res, total_res) + _tidy_up_resource_creation_idempotent(upload_state, iri, label, resource, current_res, total_res) except KeyboardInterrupt: - _tidy_up_resource_creation_idempotent(upload_state, result, resource, current_res, total_res) + _tidy_up_resource_creation_idempotent(upload_state, iri, label, resource, current_res, total_res) def _compute_counter_info_and_interrupt( @@ -395,12 +395,12 @@ def _compute_counter_info_and_interrupt( def _tidy_up_resource_creation_idempotent( upload_state: UploadState, - result: tuple[str, str] | tuple[None, None], + iri: str | None, + label: str | None, resource: XMLResource, current_res: int, total_res: int, ) -> None: - iri, label = result if iri and label: # resource creation succeeded: update the iri_resolver upload_state.iri_resolver.lookup[resource.res_id] = iri From 6652720656db5ffbaab05b993957f2a6e7c27773 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Thu, 25 Apr 2024 09:12:40 +0200 Subject: [PATCH 17/69] fix first test --- .../xmlupload/test_resource_creation.py | 33 ++++++++++++++----- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation.py b/test/integration/commands/xmlupload/test_resource_creation.py index 49ddf3488..3a65a0c3e 100644 --- a/test/integration/commands/xmlupload/test_resource_creation.py +++ b/test/integration/commands/xmlupload/test_resource_creation.py @@ -1,6 +1,7 @@ from dataclasses import dataclass -from test.integration.commands.xmlupload.connection_mock import ConnectionMockBase +from unittest.mock import Mock +import pytest from lxml import etree from dsp_tools.commands.xmlupload.iri_resolver import IriResolver @@ -39,21 +40,35 @@ def get_ontology_iri_dict(self) -> dict[str, str]: raise NotImplementedError("get_project_iri not implemented") -class ConnectionMock(ConnectionMockBase): ... - - def test_happy_path() -> None: xml_str = """ - + foo_1 text """ - xml_res = XMLResource(etree.fromstring(xml_str), "default_onto") + xml_res = XMLResource(etree.fromstring(xml_str), "my_onto") upload_state = UploadState([xml_res], [], IriResolver(), None, UploadConfig(), {}) - project_client = ProjectClientStub(ConnectionMock(), "1234", None) - _upload_resources(upload_state, ".", Sipi(ConnectionMock()), project_client, ListClientMock()) + con = Mock() + con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_label"}) + project_client = ProjectClientStub(con, "1234", None) + _upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + assert len(con.post.call_args_list) == 1 + match con.post.call_args_list[0].kwargs: + case { + "route": "/v2/resources", + "data": { + "@type": "my_onto:my_type", + "rdfs:label": "foo_label", + "knora-api:attachedToProject": {"@id": "https://admin.test.dasch.swiss/project/MsOaiQkcQ7-QPxsYBKckfQ"}, + "@context": dict(), + "my_onto:hasSimpleText": [{"@type": "knora-api:TextValue", "knora-api:valueAsString": "foo_1 text"}], + }, + }: + assert True + case _: + pytest.fail("POST request was not sent correctly") assert not upload_state.pending_resources assert not upload_state.failed_uploads - assert upload_state.iri_resolver.lookup + assert upload_state.iri_resolver.lookup["foo_1_id"] == "foo_1_iri" From 19347770778e7fb2f181b085a7f21a3733b4f72f Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Thu, 25 Apr 2024 09:56:46 +0200 Subject: [PATCH 18/69] add another test --- .../xmlupload/test_resource_creation.py | 42 ++++++++++++++++++- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation.py b/test/integration/commands/xmlupload/test_resource_creation.py index 3a65a0c3e..4cea7b125 100644 --- a/test/integration/commands/xmlupload/test_resource_creation.py +++ b/test/integration/commands/xmlupload/test_resource_creation.py @@ -40,7 +40,7 @@ def get_ontology_iri_dict(self) -> dict[str, str]: raise NotImplementedError("get_project_iri not implemented") -def test_happy_path() -> None: +def test_one_resource_without_links() -> None: xml_str = """ @@ -71,4 +71,42 @@ def test_happy_path() -> None: pytest.fail("POST request was not sent correctly") assert not upload_state.pending_resources assert not upload_state.failed_uploads - assert upload_state.iri_resolver.lookup["foo_1_id"] == "foo_1_iri" + assert upload_state.iri_resolver.lookup == {"foo_1_id": "foo_1_iri"} + assert not upload_state.pending_stash + + +def test_one_resource_with_link_to_existing_resource() -> None: + xml_str = """ + + + foo_2_id + + + """ + xml_res = XMLResource(etree.fromstring(xml_str), "my_onto") + upload_state = UploadState([xml_res], [], IriResolver({"foo_2_id": "foo_2_iri"}), None, UploadConfig(), {}) + con = Mock() + con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_label"}) + project_client = ProjectClientStub(con, "1234", None) + _upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + assert len(con.post.call_args_list) == 1 + match con.post.call_args_list[0].kwargs: + case { + "route": "/v2/resources", + "data": { + "@type": "my_onto:my_type", + "rdfs:label": "foo_label", + "knora-api:attachedToProject": {"@id": "https://admin.test.dasch.swiss/project/MsOaiQkcQ7-QPxsYBKckfQ"}, + "@context": dict(), + "my_onto:hasCustomLinkValue": [ + {"@type": "knora-api:LinkValue", "knora-api:linkValueHasTargetIri": {"@id": "foo_2_iri"}} + ], + }, + }: + assert True + case _: + pytest.fail("POST request was not sent correctly") + assert not upload_state.pending_resources + assert not upload_state.failed_uploads + assert upload_state.iri_resolver.lookup == {"foo_1_id": "foo_1_iri", "foo_2_id": "foo_2_iri"} + assert not upload_state.pending_stash From 55cbf7ea9252bc9795a92cc68ebfb0a329624544 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Thu, 25 Apr 2024 10:25:07 +0200 Subject: [PATCH 19/69] refactor --- .../commands/xmlupload/test_resource_creation.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation.py b/test/integration/commands/xmlupload/test_resource_creation.py index 4cea7b125..cface8d2b 100644 --- a/test/integration/commands/xmlupload/test_resource_creation.py +++ b/test/integration/commands/xmlupload/test_resource_creation.py @@ -42,7 +42,7 @@ def get_ontology_iri_dict(self) -> dict[str, str]: def test_one_resource_without_links() -> None: xml_str = """ - + foo_1 text @@ -51,7 +51,7 @@ def test_one_resource_without_links() -> None: xml_res = XMLResource(etree.fromstring(xml_str), "my_onto") upload_state = UploadState([xml_res], [], IriResolver(), None, UploadConfig(), {}) con = Mock() - con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_label"}) + con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}) project_client = ProjectClientStub(con, "1234", None) _upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) assert len(con.post.call_args_list) == 1 @@ -59,8 +59,8 @@ def test_one_resource_without_links() -> None: case { "route": "/v2/resources", "data": { - "@type": "my_onto:my_type", - "rdfs:label": "foo_label", + "@type": "my_onto:foo_1_type", + "rdfs:label": "foo_1_label", "knora-api:attachedToProject": {"@id": "https://admin.test.dasch.swiss/project/MsOaiQkcQ7-QPxsYBKckfQ"}, "@context": dict(), "my_onto:hasSimpleText": [{"@type": "knora-api:TextValue", "knora-api:valueAsString": "foo_1 text"}], @@ -77,7 +77,7 @@ def test_one_resource_without_links() -> None: def test_one_resource_with_link_to_existing_resource() -> None: xml_str = """ - + foo_2_id @@ -86,7 +86,7 @@ def test_one_resource_with_link_to_existing_resource() -> None: xml_res = XMLResource(etree.fromstring(xml_str), "my_onto") upload_state = UploadState([xml_res], [], IriResolver({"foo_2_id": "foo_2_iri"}), None, UploadConfig(), {}) con = Mock() - con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_label"}) + con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}) project_client = ProjectClientStub(con, "1234", None) _upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) assert len(con.post.call_args_list) == 1 @@ -94,8 +94,8 @@ def test_one_resource_with_link_to_existing_resource() -> None: case { "route": "/v2/resources", "data": { - "@type": "my_onto:my_type", - "rdfs:label": "foo_label", + "@type": "my_onto:foo_1_type", + "rdfs:label": "foo_1_label", "knora-api:attachedToProject": {"@id": "https://admin.test.dasch.swiss/project/MsOaiQkcQ7-QPxsYBKckfQ"}, "@context": dict(), "my_onto:hasCustomLinkValue": [ From 290e8ad4bdafbeb0f08a5ebc42b13d8251b21dd7 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Thu, 25 Apr 2024 10:26:54 +0200 Subject: [PATCH 20/69] test from 1 level higher --- .../commands/xmlupload/test_resource_creation.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation.py b/test/integration/commands/xmlupload/test_resource_creation.py index cface8d2b..5c8749bba 100644 --- a/test/integration/commands/xmlupload/test_resource_creation.py +++ b/test/integration/commands/xmlupload/test_resource_creation.py @@ -10,7 +10,7 @@ from dsp_tools.commands.xmlupload.models.xmlresource import XMLResource from dsp_tools.commands.xmlupload.project_client import ProjectInfo from dsp_tools.commands.xmlupload.upload_config import UploadConfig -from dsp_tools.commands.xmlupload.xmlupload import _upload_resources +from dsp_tools.commands.xmlupload.xmlupload import upload_resources from dsp_tools.utils.connection import Connection @@ -53,7 +53,7 @@ def test_one_resource_without_links() -> None: con = Mock() con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}) project_client = ProjectClientStub(con, "1234", None) - _upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) assert len(con.post.call_args_list) == 1 match con.post.call_args_list[0].kwargs: case { @@ -88,7 +88,7 @@ def test_one_resource_with_link_to_existing_resource() -> None: con = Mock() con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}) project_client = ProjectClientStub(con, "1234", None) - _upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) assert len(con.post.call_args_list) == 1 match con.post.call_args_list[0].kwargs: case { From 52e829228269891284dceead742970bf79689dd6 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Thu, 25 Apr 2024 10:29:19 +0200 Subject: [PATCH 21/69] refactor --- .../commands/xmlupload/test_resource_creation.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation.py b/test/integration/commands/xmlupload/test_resource_creation.py index 5c8749bba..288ba66dc 100644 --- a/test/integration/commands/xmlupload/test_resource_creation.py +++ b/test/integration/commands/xmlupload/test_resource_creation.py @@ -41,15 +41,17 @@ def get_ontology_iri_dict(self) -> dict[str, str]: def test_one_resource_without_links() -> None: - xml_str = """ + xml_strings = [ + """ foo_1 text """ - xml_res = XMLResource(etree.fromstring(xml_str), "my_onto") - upload_state = UploadState([xml_res], [], IriResolver(), None, UploadConfig(), {}) + ] + xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] + upload_state = UploadState(xml_resources, [], IriResolver(), None, UploadConfig(), {}) con = Mock() con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}) project_client = ProjectClientStub(con, "1234", None) @@ -76,15 +78,17 @@ def test_one_resource_without_links() -> None: def test_one_resource_with_link_to_existing_resource() -> None: - xml_str = """ + xml_strings = [ + """ foo_2_id """ - xml_res = XMLResource(etree.fromstring(xml_str), "my_onto") - upload_state = UploadState([xml_res], [], IriResolver({"foo_2_id": "foo_2_iri"}), None, UploadConfig(), {}) + ] + xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] + upload_state = UploadState(xml_resources, [], IriResolver({"foo_2_id": "foo_2_iri"}), None, UploadConfig(), {}) con = Mock() con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}) project_client = ProjectClientStub(con, "1234", None) From c04f6718baedb10f78fa038cd7ee009f1a9f85ca Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Thu, 25 Apr 2024 10:31:02 +0200 Subject: [PATCH 22/69] reformat --- .../xmlupload/test_resource_creation.py | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation.py b/test/integration/commands/xmlupload/test_resource_creation.py index 288ba66dc..94416ab34 100644 --- a/test/integration/commands/xmlupload/test_resource_creation.py +++ b/test/integration/commands/xmlupload/test_resource_creation.py @@ -43,12 +43,12 @@ def get_ontology_iri_dict(self) -> dict[str, str]: def test_one_resource_without_links() -> None: xml_strings = [ """ - - - foo_1 text - - - """ + + + foo_1 text + + + """ ] xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] upload_state = UploadState(xml_resources, [], IriResolver(), None, UploadConfig(), {}) @@ -80,12 +80,12 @@ def test_one_resource_without_links() -> None: def test_one_resource_with_link_to_existing_resource() -> None: xml_strings = [ """ - - - foo_2_id - - - """ + + + foo_2_id + + + """ ] xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] upload_state = UploadState(xml_resources, [], IriResolver({"foo_2_id": "foo_2_iri"}), None, UploadConfig(), {}) From d4db53a63dd8a94374b012b6a45a4fddb4d16b09 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Thu, 25 Apr 2024 12:33:11 +0200 Subject: [PATCH 23/69] continue --- .../xmlupload/test_resource_creation.py | 66 +++++++++++++++++-- 1 file changed, 62 insertions(+), 4 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation.py b/test/integration/commands/xmlupload/test_resource_creation.py index 94416ab34..d8b8b20ae 100644 --- a/test/integration/commands/xmlupload/test_resource_creation.py +++ b/test/integration/commands/xmlupload/test_resource_creation.py @@ -9,9 +9,14 @@ from dsp_tools.commands.xmlupload.models.upload_state import UploadState from dsp_tools.commands.xmlupload.models.xmlresource import XMLResource from dsp_tools.commands.xmlupload.project_client import ProjectInfo +from dsp_tools.commands.xmlupload.stash.stash_models import LinkValueStash +from dsp_tools.commands.xmlupload.stash.stash_models import LinkValueStashItem +from dsp_tools.commands.xmlupload.stash.stash_models import Stash from dsp_tools.commands.xmlupload.upload_config import UploadConfig from dsp_tools.commands.xmlupload.xmlupload import upload_resources +from dsp_tools.models.exceptions import PermanentTimeOutError from dsp_tools.utils.connection import Connection +from dsp_tools.utils.connection_live import ConnectionLive class ListClientMock: @@ -48,11 +53,11 @@ def test_one_resource_without_links() -> None: foo_1 text - """ + """, ] xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] upload_state = UploadState(xml_resources, [], IriResolver(), None, UploadConfig(), {}) - con = Mock() + con = Mock(spec=ConnectionLive) con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}) project_client = ProjectClientStub(con, "1234", None) upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) @@ -85,11 +90,11 @@ def test_one_resource_with_link_to_existing_resource() -> None: foo_2_id - """ + """, ] xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] upload_state = UploadState(xml_resources, [], IriResolver({"foo_2_id": "foo_2_iri"}), None, UploadConfig(), {}) - con = Mock() + con = Mock(spec=ConnectionLive) con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}) project_client = ProjectClientStub(con, "1234", None) upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) @@ -114,3 +119,56 @@ def test_one_resource_with_link_to_existing_resource() -> None: assert not upload_state.failed_uploads assert upload_state.iri_resolver.lookup == {"foo_1_id": "foo_1_iri", "foo_2_id": "foo_2_iri"} assert not upload_state.pending_stash + + +def test_one_resource_with_stashed_link() -> None: + xml_strings = [ + '', + '', + ] + xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] + link_val_stash_dict = { + "foo_1_id": [LinkValueStashItem("foo_1_id", "my_onto:foo_1_type", "my_onto:hasCustomLink", "foo_2_id")], + "foo_2_id": [LinkValueStashItem("foo_2_id", "my_onto:foo_2_type", "my_onto:hasCustomLink", "foo_1_id")], + } + stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) + upload_state = UploadState(xml_resources, [], IriResolver(), stash, UploadConfig(), {}) + con = Mock(spec=ConnectionLive) + con.post = Mock( + side_effect=[ + {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, + {"@id": "foo_2_iri", "rdfs:label": "foo_2_label"}, + ] + ) + project_client = ProjectClientStub(con, "1234", None) + upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + assert len(con.post.call_args_list) == 4 + match con.post.call_args_list[2].kwargs: + case { + "route": "/v2/values", + "data": { + "@type": "my_onto:foo_1_type", + "@id": "foo_1_iri", + "@context": dict(), + "my_onto:hasCustomLinkValue": [ + {"@type": "knora-api:LinkValue", "knora-api:linkValueHasTargetIri": {"@id": "foo_2_iri"}} + ], + }, + }: + assert True + case _: + pytest.fail("POST request was not sent correctly") + assert not upload_state.pending_resources + assert not upload_state.failed_uploads + assert upload_state.iri_resolver.lookup == {"foo_1_id": "foo_1_iri", "foo_2_id": "foo_2_iri"} + assert not upload_state.pending_stash + + +def test_two_resources_with_stash_interrupted_by_timeout() -> None: + con = Mock(spec=ConnectionLive) + con.post = Mock(side_effect=PermanentTimeOutError("")) + + +# (PermanentTimeOutError, KeyboardInterrupt) + +# interrupt_after From 85d5ea63838749855bb3d8319390e7b7efba88e8 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Thu, 25 Apr 2024 13:40:21 +0200 Subject: [PATCH 24/69] fix 3rd test --- .../commands/xmlupload/stash/stash_models.py | 6 ++++++ .../commands/xmlupload/test_resource_creation.py | 11 +++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/stash/stash_models.py b/src/dsp_tools/commands/xmlupload/stash/stash_models.py index ae1a57aff..90e1b4f5c 100644 --- a/src/dsp_tools/commands/xmlupload/stash/stash_models.py +++ b/src/dsp_tools/commands/xmlupload/stash/stash_models.py @@ -106,3 +106,9 @@ def make(standoff_stash: StandoffStash | None, link_value_stash: LinkValueStash if standoff_stash or link_value_stash: return Stash(standoff_stash, link_value_stash) return None + + def is_empty(self) -> bool: + """Check if there are any stashed items in this stash""" + standoff = not self.standoff_stash or not self.standoff_stash.res_2_stash_items + link = not self.link_value_stash or not self.link_value_stash.res_2_stash_items + return standoff and link diff --git a/test/integration/commands/xmlupload/test_resource_creation.py b/test/integration/commands/xmlupload/test_resource_creation.py index d8b8b20ae..7d3c1ba8b 100644 --- a/test/integration/commands/xmlupload/test_resource_creation.py +++ b/test/integration/commands/xmlupload/test_resource_creation.py @@ -138,6 +138,8 @@ def test_one_resource_with_stashed_link() -> None: side_effect=[ {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, {"@id": "foo_2_iri", "rdfs:label": "foo_2_label"}, + {}, + {}, ] ) project_client = ProjectClientStub(con, "1234", None) @@ -150,9 +152,10 @@ def test_one_resource_with_stashed_link() -> None: "@type": "my_onto:foo_1_type", "@id": "foo_1_iri", "@context": dict(), - "my_onto:hasCustomLinkValue": [ - {"@type": "knora-api:LinkValue", "knora-api:linkValueHasTargetIri": {"@id": "foo_2_iri"}} - ], + "my_onto:hasCustomLinkValue": { + "@type": "knora-api:LinkValue", + "knora-api:linkValueHasTargetIri": {"@id": "foo_2_iri"}, + }, }, }: assert True @@ -161,7 +164,7 @@ def test_one_resource_with_stashed_link() -> None: assert not upload_state.pending_resources assert not upload_state.failed_uploads assert upload_state.iri_resolver.lookup == {"foo_1_id": "foo_1_iri", "foo_2_id": "foo_2_iri"} - assert not upload_state.pending_stash + assert not upload_state.pending_stash or upload_state.pending_stash.is_empty() def test_two_resources_with_stash_interrupted_by_timeout() -> None: From a268f0963d291be6182ff4942402c661b21475a0 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Thu, 25 Apr 2024 14:00:39 +0200 Subject: [PATCH 25/69] 4th test --- .../xmlupload/test_resource_creation.py | 41 ++++++++++++++++--- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation.py b/test/integration/commands/xmlupload/test_resource_creation.py index 7d3c1ba8b..ecf21ca7e 100644 --- a/test/integration/commands/xmlupload/test_resource_creation.py +++ b/test/integration/commands/xmlupload/test_resource_creation.py @@ -1,9 +1,11 @@ +from copy import copy from dataclasses import dataclass from unittest.mock import Mock import pytest from lxml import etree +from dsp_tools.commands.xmlupload import xmlupload from dsp_tools.commands.xmlupload.iri_resolver import IriResolver from dsp_tools.commands.xmlupload.models.sipi import Sipi from dsp_tools.commands.xmlupload.models.upload_state import UploadState @@ -13,8 +15,8 @@ from dsp_tools.commands.xmlupload.stash.stash_models import LinkValueStashItem from dsp_tools.commands.xmlupload.stash.stash_models import Stash from dsp_tools.commands.xmlupload.upload_config import UploadConfig -from dsp_tools.commands.xmlupload.xmlupload import upload_resources from dsp_tools.models.exceptions import PermanentTimeOutError +from dsp_tools.models.exceptions import XmlUploadInterruptedError from dsp_tools.utils.connection import Connection from dsp_tools.utils.connection_live import ConnectionLive @@ -60,7 +62,7 @@ def test_one_resource_without_links() -> None: con = Mock(spec=ConnectionLive) con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}) project_client = ProjectClientStub(con, "1234", None) - upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) assert len(con.post.call_args_list) == 1 match con.post.call_args_list[0].kwargs: case { @@ -97,7 +99,7 @@ def test_one_resource_with_link_to_existing_resource() -> None: con = Mock(spec=ConnectionLive) con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}) project_client = ProjectClientStub(con, "1234", None) - upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) assert len(con.post.call_args_list) == 1 match con.post.call_args_list[0].kwargs: case { @@ -121,7 +123,7 @@ def test_one_resource_with_link_to_existing_resource() -> None: assert not upload_state.pending_stash -def test_one_resource_with_stashed_link() -> None: +def test_twos_resource_with_stashed_link() -> None: xml_strings = [ '', '', @@ -143,7 +145,7 @@ def test_one_resource_with_stashed_link() -> None: ] ) project_client = ProjectClientStub(con, "1234", None) - upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) assert len(con.post.call_args_list) == 4 match con.post.call_args_list[2].kwargs: case { @@ -168,8 +170,35 @@ def test_one_resource_with_stashed_link() -> None: def test_two_resources_with_stash_interrupted_by_timeout() -> None: + xml_strings = [ + '', + '', + ] + xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] + link_val_stash_dict = { + "foo_1_id": [LinkValueStashItem("foo_1_id", "my_onto:foo_1_type", "my_onto:hasCustomLink", "foo_2_id")], + "foo_2_id": [LinkValueStashItem("foo_2_id", "my_onto:foo_2_type", "my_onto:hasCustomLink", "foo_1_id")], + } + stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) + upload_state = UploadState(xml_resources.copy(), [], IriResolver(), copy(stash), UploadConfig(), {}) con = Mock(spec=ConnectionLive) - con.post = Mock(side_effect=PermanentTimeOutError("")) + con.post = Mock(side_effect=[{"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, PermanentTimeOutError("")]) + handle_upload_error_mock = Mock() + xmlupload._handle_upload_error = handle_upload_error_mock + project_client = ProjectClientStub(con, "1234", None) + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + err_msg = ( + "There was a PermanentTimeOutError while trying to create resource 'foo_2_id'.\n" + "It is unclear if the resource 'foo_2_id' was created successfully or not.\n" + "Please check manually in the DSP-APP or DB.\n" + "In case of successful creation, call 'resume-xmlupload' with the flag " + "'--skip-first-resource' to prevent duplication.\n" + "If not, a normal 'resume-xmlupload' can be started." + ) + upload_state_expected = UploadState( + xml_resources[1:], [], IriResolver({"foo_1_id": "foo_1_iri"}), stash, UploadConfig(), {} + ) + handle_upload_error_mock.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) # (PermanentTimeOutError, KeyboardInterrupt) From bd19b06af7175b606e3d00a9f79cb135d70ab2ff Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Thu, 25 Apr 2024 14:23:11 +0200 Subject: [PATCH 26/69] add another test --- .../xmlupload/test_resource_creation.py | 34 +++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation.py b/test/integration/commands/xmlupload/test_resource_creation.py index ecf21ca7e..b15781fa0 100644 --- a/test/integration/commands/xmlupload/test_resource_creation.py +++ b/test/integration/commands/xmlupload/test_resource_creation.py @@ -123,7 +123,7 @@ def test_one_resource_with_link_to_existing_resource() -> None: assert not upload_state.pending_stash -def test_twos_resource_with_stashed_link() -> None: +def test_two_resources_with_stash() -> None: xml_strings = [ '', '', @@ -201,6 +201,36 @@ def test_two_resources_with_stash_interrupted_by_timeout() -> None: handle_upload_error_mock.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) -# (PermanentTimeOutError, KeyboardInterrupt) +def test_two_resources_with_stash_interrupted_by_keyboard() -> None: + xml_strings = [ + '', + '', + ] + xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] + link_val_stash_dict = { + "foo_1_id": [LinkValueStashItem("foo_1_id", "my_onto:foo_1_type", "my_onto:hasCustomLink", "foo_2_id")], + "foo_2_id": [LinkValueStashItem("foo_2_id", "my_onto:foo_2_type", "my_onto:hasCustomLink", "foo_1_id")], + } + stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) + upload_state = UploadState(xml_resources.copy(), [], IriResolver(), copy(stash), UploadConfig(), {}) + con = Mock(spec=ConnectionLive) + con.post = Mock(side_effect=[{"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, KeyboardInterrupt()]) + handle_upload_error_mock = Mock() + xmlupload._handle_upload_error = handle_upload_error_mock + project_client = ProjectClientStub(con, "1234", None) + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + err_msg = ( + "There was a KeyboardInterrupt while trying to create resource 'foo_2_id'.\n" + "It is unclear if the resource 'foo_2_id' was created successfully or not.\n" + "Please check manually in the DSP-APP or DB.\n" + "In case of successful creation, call 'resume-xmlupload' with the flag " + "'--skip-first-resource' to prevent duplication.\n" + "If not, a normal 'resume-xmlupload' can be started." + ) + upload_state_expected = UploadState( + xml_resources[1:], [], IriResolver({"foo_1_id": "foo_1_iri"}), stash, UploadConfig(), {} + ) + handle_upload_error_mock.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) + # interrupt_after From 557794e981b25a24fc5ff19ad99315c46684d027 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Thu, 25 Apr 2024 14:24:16 +0200 Subject: [PATCH 27/69] new file + rename --- ...esource_creation.py => test_resource_creation_integration.py} | 0 test/unittests/commands/xmlupload/test_resource_creation_unit.py | 1 + 2 files changed, 1 insertion(+) rename test/integration/commands/xmlupload/{test_resource_creation.py => test_resource_creation_integration.py} (100%) create mode 100644 test/unittests/commands/xmlupload/test_resource_creation_unit.py diff --git a/test/integration/commands/xmlupload/test_resource_creation.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py similarity index 100% rename from test/integration/commands/xmlupload/test_resource_creation.py rename to test/integration/commands/xmlupload/test_resource_creation_integration.py diff --git a/test/unittests/commands/xmlupload/test_resource_creation_unit.py b/test/unittests/commands/xmlupload/test_resource_creation_unit.py new file mode 100644 index 000000000..a51e139cd --- /dev/null +++ b/test/unittests/commands/xmlupload/test_resource_creation_unit.py @@ -0,0 +1 @@ +# test idempotency From 09caba70fa42bc9486fb3b921ce759f06f441dcc Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Thu, 25 Apr 2024 14:40:25 +0200 Subject: [PATCH 28/69] add unit tests for idempotency --- .../xmlupload/test_resource_creation_unit.py | 54 ++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/test/unittests/commands/xmlupload/test_resource_creation_unit.py b/test/unittests/commands/xmlupload/test_resource_creation_unit.py index a51e139cd..16e9a148c 100644 --- a/test/unittests/commands/xmlupload/test_resource_creation_unit.py +++ b/test/unittests/commands/xmlupload/test_resource_creation_unit.py @@ -1 +1,53 @@ -# test idempotency +from lxml import etree + +from dsp_tools.commands.xmlupload.iri_resolver import IriResolver +from dsp_tools.commands.xmlupload.models.upload_state import UploadState +from dsp_tools.commands.xmlupload.models.xmlresource import XMLResource +from dsp_tools.commands.xmlupload.upload_config import UploadConfig +from dsp_tools.commands.xmlupload.xmlupload import _tidy_up_resource_creation_idempotent + + +def test_idempotency_on_success() -> None: + xml_strings = [ + """ + + foo_1 text + + """, + """ + + foo_2 text + + """, + ] + xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] + upload_state = UploadState(xml_resources.copy(), [], IriResolver(), None, UploadConfig(), {}) + for _ in range(3): + _tidy_up_resource_creation_idempotent(upload_state, "foo_1_iri", "foo_1_label", xml_resources[0], 1, 2) + assert upload_state.pending_resources == xml_resources[1:] + assert upload_state.failed_uploads == [] + assert upload_state.iri_resolver.lookup == {"foo_1_id": "foo_1_iri"} + assert not upload_state.pending_stash + + +def test_idempotency_on_failure() -> None: + xml_strings = [ + """ + + foo_1 text + + """, + """ + + foo_2 text + + """, + ] + xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] + upload_state = UploadState(xml_resources.copy(), [], IriResolver(), None, UploadConfig(), {}) + for _ in range(3): + _tidy_up_resource_creation_idempotent(upload_state, None, None, xml_resources[0], 1, 2) + assert upload_state.pending_resources == xml_resources[1:] + assert upload_state.failed_uploads == ["foo_1_id"] + assert upload_state.iri_resolver.lookup == {} + assert not upload_state.pending_stash From 7143bd6f833b5dcb9c96e4e4e91d5ded44f75ec9 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Thu, 25 Apr 2024 15:32:18 +0200 Subject: [PATCH 29/69] interrupt_after: still failing --- .../test_resource_creation_integration.py | 65 +++++++++++++++---- 1 file changed, 52 insertions(+), 13 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index b15781fa0..233063a36 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -51,9 +51,7 @@ def test_one_resource_without_links() -> None: xml_strings = [ """ - - foo_1 text - + foo_1 text """, ] @@ -88,9 +86,7 @@ def test_one_resource_with_link_to_existing_resource() -> None: xml_strings = [ """ - - foo_2_id - + foo_2_id """, ] @@ -183,8 +179,7 @@ def test_two_resources_with_stash_interrupted_by_timeout() -> None: upload_state = UploadState(xml_resources.copy(), [], IriResolver(), copy(stash), UploadConfig(), {}) con = Mock(spec=ConnectionLive) con.post = Mock(side_effect=[{"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, PermanentTimeOutError("")]) - handle_upload_error_mock = Mock() - xmlupload._handle_upload_error = handle_upload_error_mock + xmlupload._handle_upload_error = Mock() project_client = ProjectClientStub(con, "1234", None) xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) err_msg = ( @@ -198,7 +193,7 @@ def test_two_resources_with_stash_interrupted_by_timeout() -> None: upload_state_expected = UploadState( xml_resources[1:], [], IriResolver({"foo_1_id": "foo_1_iri"}), stash, UploadConfig(), {} ) - handle_upload_error_mock.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) + xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) def test_two_resources_with_stash_interrupted_by_keyboard() -> None: @@ -215,8 +210,7 @@ def test_two_resources_with_stash_interrupted_by_keyboard() -> None: upload_state = UploadState(xml_resources.copy(), [], IriResolver(), copy(stash), UploadConfig(), {}) con = Mock(spec=ConnectionLive) con.post = Mock(side_effect=[{"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, KeyboardInterrupt()]) - handle_upload_error_mock = Mock() - xmlupload._handle_upload_error = handle_upload_error_mock + xmlupload._handle_upload_error = Mock() project_client = ProjectClientStub(con, "1234", None) xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) err_msg = ( @@ -230,7 +224,52 @@ def test_two_resources_with_stash_interrupted_by_keyboard() -> None: upload_state_expected = UploadState( xml_resources[1:], [], IriResolver({"foo_1_id": "foo_1_iri"}), stash, UploadConfig(), {} ) - handle_upload_error_mock.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) + xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) -# interrupt_after +def test_two_resources_with_stash_interrupt_after() -> None: + xml_strings = [ + '', + '', + '', + '', + '', + '', + ] + xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] + link_val_stash_dict = { + "foo_1_id": [LinkValueStashItem("foo_1_id", "my_onto:foo_1_type", "my_onto:hasCustomLink", "foo_2_id")], + "foo_2_id": [LinkValueStashItem("foo_2_id", "my_onto:foo_2_type", "my_onto:hasCustomLink", "foo_1_id")], + } + stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) + upload_config = UploadConfig(interrupt_after=2) + upload_state = UploadState(xml_resources.copy(), [], IriResolver(), copy(stash), upload_config, {}) + con = Mock(spec=ConnectionLive) + con.post = Mock( + side_effect=[ + {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, + {"@id": "foo_2_iri", "rdfs:label": "foo_2_label"}, + {"@id": "foo_3_iri", "rdfs:label": "foo_3_label"}, + {"@id": "foo_4_iri", "rdfs:label": "foo_4_label"}, + {"@id": "foo_5_iri", "rdfs:label": "foo_5_label"}, + {"@id": "foo_6_iri", "rdfs:label": "foo_6_label"}, + ] + ) + xmlupload._handle_upload_error = Mock() + project_client = ProjectClientStub(con, "1234", None) + err_msg = "Interrupted: Maximum number of resources was reached (2)" + + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + iri_resolver_dict = {"foo_1_id": "foo_1_iri", "foo_2_id": "foo_2_iri"} + upload_state_expected = UploadState(xml_resources[2:], [], IriResolver(iri_resolver_dict), stash, upload_config, {}) + xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) + + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + iri_resolver_dict.update({"foo_3_id": "foo_3_iri", "foo_4_id": "foo_4_iri"}) + upload_state_expected = UploadState(xml_resources[4:], [], IriResolver(iri_resolver_dict), stash, upload_config, {}) + xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) + + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + iri_resolver_dict.update({"foo_5_id": "foo_5_iri", "foo_6_id": "foo_6_iri"}) + upload_state_expected = UploadState(xml_resources[4:], [], IriResolver(iri_resolver_dict), stash, upload_config, {}) + xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) From 350fe8fac4ca19069e906d61cf0f5ecb748bc71b Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Thu, 25 Apr 2024 15:33:05 +0200 Subject: [PATCH 30/69] fix attempt --- src/dsp_tools/commands/xmlupload/xmlupload.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index c89255cf2..dc330c20f 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -350,7 +350,7 @@ def _upload_one_resource( resource_create_client: ResourceCreateClient, previous_total: int, ) -> None: - current_res, total_res = _compute_counter_info_and_interrupt(upload_state, previous_total, resource) + current_res, total_res = _compute_counter_info_and_interrupt(upload_state, previous_total) success, media_info = handle_media_info( resource, upload_state.config.media_previously_uploaded, sipi_server, imgdir, upload_state.permissions_lookup ) @@ -378,19 +378,17 @@ def _upload_one_resource( _tidy_up_resource_creation_idempotent(upload_state, iri, label, resource, current_res, total_res) -def _compute_counter_info_and_interrupt( - upload_state: UploadState, previous_total: int, resource: XMLResource -) -> tuple[int, int]: - total_res = len(upload_state.pending_resources) + len(upload_state.iri_resolver.lookup) - counter = upload_state.pending_resources.index(resource) - current_res = counter + 1 + previous_total +def _compute_counter_info_and_interrupt(upload_state: UploadState, previous_total: int) -> tuple[int, int]: + creation_attempts_in_current_run = len(upload_state.iri_resolver.lookup) + len(upload_state.failed_uploads) + total_res_in_current_run = creation_attempts_in_current_run + len(upload_state.pending_resources) + current_res = creation_attempts_in_current_run + 1 + previous_total # if the interrupt_after value is not set, the upload will not be interrupted - interrupt_after = upload_state.config.interrupt_after or total_res + 1 - if counter >= interrupt_after: + interrupt_after = upload_state.config.interrupt_after or total_res_in_current_run + 1 + if creation_attempts_in_current_run >= interrupt_after: raise XmlUploadInterruptedError( f"Interrupted: Maximum number of resources was reached ({upload_state.config.interrupt_after})" ) - return current_res, total_res + return current_res, total_res_in_current_run def _tidy_up_resource_creation_idempotent( From 8f8aec07c822fa8988fe835e55738936a64c6d65 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Fri, 26 Apr 2024 08:47:42 +0200 Subject: [PATCH 31/69] continue --- .pre-commit-config.yaml | 1 + src/dsp_tools/commands/xmlupload/xmlupload.py | 25 +++++++------------ .../test_resource_creation_integration.py | 2 ++ 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 54c233a7d..ca991fcdf 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -13,6 +13,7 @@ repos: --ignore=PLR0913, --ignore=PLR2004, --ignore=PLW0603, + --ignore=FIX002, ] - id: ruff-format diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index dc330c20f..ae7500c4e 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -316,7 +316,6 @@ def _upload_resources( project_iri = project_client.get_project_iri() json_ld_context = get_json_ld_context_for_project(project_client.get_ontology_name_dict()) listnode_lookup = list_client.get_list_node_id_to_iri_lookup() - resource_create_client = ResourceCreateClient( con=project_client.con, project_iri=project_iri, @@ -326,11 +325,6 @@ def _upload_resources( listnode_lookup=listnode_lookup, media_previously_ingested=upload_state.config.media_previously_uploaded, ) - - previous_successful = len(upload_state.iri_resolver.lookup) - previous_failed = len(upload_state.failed_uploads) - previous_total = previous_successful + previous_failed - for resource in upload_state.pending_resources.copy(): _upload_one_resource( upload_state=upload_state, @@ -338,7 +332,6 @@ def _upload_resources( imgdir=imgdir, sipi_server=sipi_server, resource_create_client=resource_create_client, - previous_total=previous_total, ) @@ -348,9 +341,8 @@ def _upload_one_resource( imgdir: str, sipi_server: Sipi, resource_create_client: ResourceCreateClient, - previous_total: int, ) -> None: - current_res, total_res = _compute_counter_info_and_interrupt(upload_state, previous_total) + current_res, total_res = _compute_counter_info_and_interrupt(upload_state) success, media_info = handle_media_info( resource, upload_state.config.media_previously_uploaded, sipi_server, imgdir, upload_state.permissions_lookup ) @@ -378,17 +370,18 @@ def _upload_one_resource( _tidy_up_resource_creation_idempotent(upload_state, iri, label, resource, current_res, total_res) -def _compute_counter_info_and_interrupt(upload_state: UploadState, previous_total: int) -> tuple[int, int]: - creation_attempts_in_current_run = len(upload_state.iri_resolver.lookup) + len(upload_state.failed_uploads) - total_res_in_current_run = creation_attempts_in_current_run + len(upload_state.pending_resources) - current_res = creation_attempts_in_current_run + 1 + previous_total +def _compute_counter_info_and_interrupt(upload_state: UploadState) -> tuple[int, int]: + previous_creation_attempts = len(upload_state.iri_resolver.lookup) + len(upload_state.failed_uploads) + total_num_of_resources = previous_creation_attempts + len(upload_state.pending_resources) + num_of_current_res = previous_creation_attempts + 1 + num_of_current_res_in_this_round = num_of_current_res - previous_creation_attempts # if the interrupt_after value is not set, the upload will not be interrupted - interrupt_after = upload_state.config.interrupt_after or total_res_in_current_run + 1 - if creation_attempts_in_current_run >= interrupt_after: + interrupt_after = upload_state.config.interrupt_after or total_num_of_resources + 1 + if num_of_current_res_in_this_round >= interrupt_after: raise XmlUploadInterruptedError( f"Interrupted: Maximum number of resources was reached ({upload_state.config.interrupt_after})" ) - return current_res, total_res_in_current_run + return num_of_current_res, total_num_of_resources def _tidy_up_resource_creation_idempotent( diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index 233063a36..52efd5125 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -273,3 +273,5 @@ def test_two_resources_with_stash_interrupt_after() -> None: iri_resolver_dict.update({"foo_5_id": "foo_5_iri", "foo_6_id": "foo_6_iri"}) upload_state_expected = UploadState(xml_resources[4:], [], IriResolver(iri_resolver_dict), stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) + + # TODO: check if logs have been emitted with the correct numbering/counting From 464a08f02fb79c37948342ab9bf12b17af93c464 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Fri, 26 Apr 2024 09:32:20 +0200 Subject: [PATCH 32/69] add warning that tidy up is in progress --- src/dsp_tools/commands/xmlupload/xmlupload.py | 4 ++++ src/dsp_tools/models/custom_warnings.py | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index ae7500c4e..f6d762ce1 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -2,6 +2,7 @@ import pickle import sys +import warnings from datetime import datetime from pathlib import Path from typing import Any @@ -32,6 +33,7 @@ from dsp_tools.commands.xmlupload.stash.upload_stashed_xml_texts import upload_stashed_xml_texts from dsp_tools.commands.xmlupload.upload_config import UploadConfig from dsp_tools.commands.xmlupload.write_diagnostic_info import write_id2iri_mapping +from dsp_tools.models.custom_warnings import GeneralDspToolsWarning from dsp_tools.models.exceptions import BaseError from dsp_tools.models.exceptions import PermanentTimeOutError from dsp_tools.models.exceptions import UserError @@ -353,6 +355,7 @@ def _upload_one_resource( try: iri, label = _create_resource(resource, media_info, resource_create_client) except (PermanentTimeOutError, KeyboardInterrupt) as err: + warnings.warn(GeneralDspToolsWarning(f"{type(err).__name__}: Tidying up, then exit...")) msg = ( f"There was a {type(err).__name__} while trying to create resource '{resource.res_id}'.\n" f"It is unclear if the resource '{resource.res_id}' was created successfully or not.\n" @@ -367,6 +370,7 @@ def _upload_one_resource( try: _tidy_up_resource_creation_idempotent(upload_state, iri, label, resource, current_res, total_res) except KeyboardInterrupt: + warnings.warn(GeneralDspToolsWarning("KeyboardInterrupt: Tidying up, then exit...")) _tidy_up_resource_creation_idempotent(upload_state, iri, label, resource, current_res, total_res) diff --git a/src/dsp_tools/models/custom_warnings.py b/src/dsp_tools/models/custom_warnings.py index a7b22329c..5ab9cab25 100644 --- a/src/dsp_tools/models/custom_warnings.py +++ b/src/dsp_tools/models/custom_warnings.py @@ -13,6 +13,15 @@ def showwarning(cls, message: str) -> None: """Functionality that should be executed when a warning of this class is emitted""" +class GeneralDspToolsWarning(Warning): + """Class for user-facing deprecation warnings""" + + @classmethod + def showwarning(cls, message: str) -> None: + """Print the warning, without context""" + cprint(message, color="red", attrs=["bold"]) + + class DspToolsFutureWarning(FutureWarning): """Class for user-facing deprecation warnings""" From 10d11885e9b849632e2fd1e70bbd54d44fad93c2 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Fri, 26 Apr 2024 10:16:22 +0200 Subject: [PATCH 33/69] edit --- .../commands/xmlupload/test_resource_creation_integration.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index 52efd5125..939e80e31 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -275,3 +275,5 @@ def test_two_resources_with_stash_interrupt_after() -> None: xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) # TODO: check if logs have been emitted with the correct numbering/counting + + # TODO: check that warnings have been emitted From f5ede09a0abcbd4a28e173b34fe90a64dba3e5e6 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Fri, 26 Apr 2024 12:24:10 +0200 Subject: [PATCH 34/69] fix? --- src/dsp_tools/commands/xmlupload/xmlupload.py | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index f6d762ce1..9d66ec384 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -318,6 +318,7 @@ def _upload_resources( project_iri = project_client.get_project_iri() json_ld_context = get_json_ld_context_for_project(project_client.get_ontology_name_dict()) listnode_lookup = list_client.get_list_node_id_to_iri_lookup() + resource_create_client = ResourceCreateClient( con=project_client.con, project_iri=project_iri, @@ -327,13 +328,20 @@ def _upload_resources( listnode_lookup=listnode_lookup, media_previously_ingested=upload_state.config.media_previously_uploaded, ) - for resource in upload_state.pending_resources.copy(): + previous_successful = len(upload_state.iri_resolver.lookup) + previous_failed = len(upload_state.failed_uploads) + upcoming = len(upload_state.pending_resources) + total_num_of_resources = previous_successful + previous_failed + upcoming + + for creation_attempts_of_this_round, resource in enumerate(upload_state.pending_resources.copy()): _upload_one_resource( upload_state=upload_state, resource=resource, imgdir=imgdir, sipi_server=sipi_server, resource_create_client=resource_create_client, + total_num_of_resources=total_num_of_resources, + creation_attempts_of_this_round=creation_attempts_of_this_round, ) @@ -343,8 +351,10 @@ def _upload_one_resource( imgdir: str, sipi_server: Sipi, resource_create_client: ResourceCreateClient, + total_num_of_resources: int, + creation_attempts_of_this_round: int, ) -> None: - current_res, total_res = _compute_counter_info_and_interrupt(upload_state) + _compute_counter_info_and_interrupt(upload_state, total_num_of_resources, creation_attempts_of_this_round) success, media_info = handle_media_info( resource, upload_state.config.media_previously_uploaded, sipi_server, imgdir, upload_state.permissions_lookup ) @@ -368,24 +378,25 @@ def _upload_one_resource( raise XmlUploadInterruptedError(msg) from None try: - _tidy_up_resource_creation_idempotent(upload_state, iri, label, resource, current_res, total_res) + _tidy_up_resource_creation_idempotent( + upload_state, iri, label, resource, creation_attempts_of_this_round + 1, total_num_of_resources + ) except KeyboardInterrupt: warnings.warn(GeneralDspToolsWarning("KeyboardInterrupt: Tidying up, then exit...")) - _tidy_up_resource_creation_idempotent(upload_state, iri, label, resource, current_res, total_res) + _tidy_up_resource_creation_idempotent( + upload_state, iri, label, resource, creation_attempts_of_this_round + 1, total_num_of_resources + ) -def _compute_counter_info_and_interrupt(upload_state: UploadState) -> tuple[int, int]: - previous_creation_attempts = len(upload_state.iri_resolver.lookup) + len(upload_state.failed_uploads) - total_num_of_resources = previous_creation_attempts + len(upload_state.pending_resources) - num_of_current_res = previous_creation_attempts + 1 - num_of_current_res_in_this_round = num_of_current_res - previous_creation_attempts +def _compute_counter_info_and_interrupt( + upload_state: UploadState, total_num_of_resources: int, creation_attempts_of_this_round: int +) -> None: # if the interrupt_after value is not set, the upload will not be interrupted interrupt_after = upload_state.config.interrupt_after or total_num_of_resources + 1 - if num_of_current_res_in_this_round >= interrupt_after: + if creation_attempts_of_this_round >= interrupt_after: raise XmlUploadInterruptedError( f"Interrupted: Maximum number of resources was reached ({upload_state.config.interrupt_after})" ) - return num_of_current_res, total_num_of_resources def _tidy_up_resource_creation_idempotent( From bc7feecd5a5889ed69e92371e2cedaaa7143a2d9 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 29 Apr 2024 07:54:48 +0200 Subject: [PATCH 35/69] fix + rename --- .../test_resource_creation_integration.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index 939e80e31..f8c837669 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -260,18 +260,20 @@ def test_two_resources_with_stash_interrupt_after() -> None: err_msg = "Interrupted: Maximum number of resources was reached (2)" xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) - iri_resolver_dict = {"foo_1_id": "foo_1_iri", "foo_2_id": "foo_2_iri"} - upload_state_expected = UploadState(xml_resources[2:], [], IriResolver(iri_resolver_dict), stash, upload_config, {}) + iri_resolver_expected = IriResolver({"foo_1_id": "foo_1_iri", "foo_2_id": "foo_2_iri"}) + upload_state_expected = UploadState(xml_resources[2:], [], iri_resolver_expected, stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) + xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) - iri_resolver_dict.update({"foo_3_id": "foo_3_iri", "foo_4_id": "foo_4_iri"}) - upload_state_expected = UploadState(xml_resources[4:], [], IriResolver(iri_resolver_dict), stash, upload_config, {}) + iri_resolver_expected.lookup.update({"foo_3_id": "foo_3_iri", "foo_4_id": "foo_4_iri"}) + upload_state_expected = UploadState(xml_resources[4:], [], iri_resolver_expected, stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) + xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) - iri_resolver_dict.update({"foo_5_id": "foo_5_iri", "foo_6_id": "foo_6_iri"}) - upload_state_expected = UploadState(xml_resources[4:], [], IriResolver(iri_resolver_dict), stash, upload_config, {}) + iri_resolver_expected.lookup.update({"foo_5_id": "foo_5_iri", "foo_6_id": "foo_6_iri"}) + upload_state_expected = UploadState(xml_resources[4:], [], iri_resolver_expected, stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) # TODO: check if logs have been emitted with the correct numbering/counting From 8ebc154e59e4572837a92bda7165c3b01a37adfe Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 29 Apr 2024 08:41:58 +0200 Subject: [PATCH 36/69] fix --- src/dsp_tools/commands/xmlupload/xmlupload.py | 7 ++++--- .../xmlupload/test_resource_creation_integration.py | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index 9d66ec384..7e410e4fb 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -354,7 +354,6 @@ def _upload_one_resource( total_num_of_resources: int, creation_attempts_of_this_round: int, ) -> None: - _compute_counter_info_and_interrupt(upload_state, total_num_of_resources, creation_attempts_of_this_round) success, media_info = handle_media_info( resource, upload_state.config.media_previously_uploaded, sipi_server, imgdir, upload_state.permissions_lookup ) @@ -387,13 +386,15 @@ def _upload_one_resource( upload_state, iri, label, resource, creation_attempts_of_this_round + 1, total_num_of_resources ) + _interrupt_if_indicated(upload_state, total_num_of_resources, creation_attempts_of_this_round) -def _compute_counter_info_and_interrupt( + +def _interrupt_if_indicated( upload_state: UploadState, total_num_of_resources: int, creation_attempts_of_this_round: int ) -> None: # if the interrupt_after value is not set, the upload will not be interrupted interrupt_after = upload_state.config.interrupt_after or total_num_of_resources + 1 - if creation_attempts_of_this_round >= interrupt_after: + if creation_attempts_of_this_round + 1 >= interrupt_after: raise XmlUploadInterruptedError( f"Interrupted: Maximum number of resources was reached ({upload_state.config.interrupt_after})" ) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index f8c837669..b24f3aa86 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -273,7 +273,7 @@ def test_two_resources_with_stash_interrupt_after() -> None: xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) iri_resolver_expected.lookup.update({"foo_5_id": "foo_5_iri", "foo_6_id": "foo_6_iri"}) - upload_state_expected = UploadState(xml_resources[4:], [], iri_resolver_expected, stash, upload_config, {}) + upload_state_expected = UploadState([], [], iri_resolver_expected, stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) # TODO: check if logs have been emitted with the correct numbering/counting From bcaf65512a92a8245148c15cb56349aff04aab19 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 29 Apr 2024 08:42:34 +0200 Subject: [PATCH 37/69] fix linter --- .../commands/xmlupload/test_resource_creation_integration.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index b24f3aa86..766d0d521 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -275,7 +275,3 @@ def test_two_resources_with_stash_interrupt_after() -> None: iri_resolver_expected.lookup.update({"foo_5_id": "foo_5_iri", "foo_6_id": "foo_6_iri"}) upload_state_expected = UploadState([], [], iri_resolver_expected, stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) - - # TODO: check if logs have been emitted with the correct numbering/counting - - # TODO: check that warnings have been emitted From 2a1a422eb5c3861b8a968cfe64d6647b0cd77776 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 29 Apr 2024 10:23:00 +0200 Subject: [PATCH 38/69] factor out reusable part --- .../test_resource_creation_integration.py | 37 ++++--------------- 1 file changed, 7 insertions(+), 30 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index 766d0d521..1c5c3e15b 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -166,37 +166,14 @@ def test_two_resources_with_stash() -> None: def test_two_resources_with_stash_interrupted_by_timeout() -> None: - xml_strings = [ - '', - '', - ] - xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] - link_val_stash_dict = { - "foo_1_id": [LinkValueStashItem("foo_1_id", "my_onto:foo_1_type", "my_onto:hasCustomLink", "foo_2_id")], - "foo_2_id": [LinkValueStashItem("foo_2_id", "my_onto:foo_2_type", "my_onto:hasCustomLink", "foo_1_id")], - } - stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) - upload_state = UploadState(xml_resources.copy(), [], IriResolver(), copy(stash), UploadConfig(), {}) - con = Mock(spec=ConnectionLive) - con.post = Mock(side_effect=[{"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, PermanentTimeOutError("")]) - xmlupload._handle_upload_error = Mock() - project_client = ProjectClientStub(con, "1234", None) - xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) - err_msg = ( - "There was a PermanentTimeOutError while trying to create resource 'foo_2_id'.\n" - "It is unclear if the resource 'foo_2_id' was created successfully or not.\n" - "Please check manually in the DSP-APP or DB.\n" - "In case of successful creation, call 'resume-xmlupload' with the flag " - "'--skip-first-resource' to prevent duplication.\n" - "If not, a normal 'resume-xmlupload' can be started." - ) - upload_state_expected = UploadState( - xml_resources[1:], [], IriResolver({"foo_1_id": "foo_1_iri"}), stash, UploadConfig(), {} - ) - xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) + _two_resources_with_stash_interrupted_by_error(PermanentTimeOutError(""), "PermanentTimeOutError") def test_two_resources_with_stash_interrupted_by_keyboard() -> None: + _two_resources_with_stash_interrupted_by_error(KeyboardInterrupt(), "KeyboardInterrupt") + + +def _two_resources_with_stash_interrupted_by_error(err_to_interrupt_with: BaseException, err_as_str: str) -> None: xml_strings = [ '', '', @@ -209,12 +186,12 @@ def test_two_resources_with_stash_interrupted_by_keyboard() -> None: stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) upload_state = UploadState(xml_resources.copy(), [], IriResolver(), copy(stash), UploadConfig(), {}) con = Mock(spec=ConnectionLive) - con.post = Mock(side_effect=[{"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, KeyboardInterrupt()]) + con.post = Mock(side_effect=[{"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, err_to_interrupt_with]) xmlupload._handle_upload_error = Mock() project_client = ProjectClientStub(con, "1234", None) xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) err_msg = ( - "There was a KeyboardInterrupt while trying to create resource 'foo_2_id'.\n" + f"There was a {err_as_str} while trying to create resource 'foo_2_id'.\n" "It is unclear if the resource 'foo_2_id' was created successfully or not.\n" "Please check manually in the DSP-APP or DB.\n" "In case of successful creation, call 'resume-xmlupload' with the flag " From 03b0ad34a7c3e6362a2e2ca847ecfeea3c25e4a7 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 29 Apr 2024 10:31:26 +0200 Subject: [PATCH 39/69] use spec_set instead of spec --- .../xmlupload/test_resource_creation_integration.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index 1c5c3e15b..ee9af9dbf 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -57,7 +57,7 @@ def test_one_resource_without_links() -> None: ] xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] upload_state = UploadState(xml_resources, [], IriResolver(), None, UploadConfig(), {}) - con = Mock(spec=ConnectionLive) + con = Mock(spec_set=ConnectionLive) con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}) project_client = ProjectClientStub(con, "1234", None) xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) @@ -92,7 +92,7 @@ def test_one_resource_with_link_to_existing_resource() -> None: ] xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] upload_state = UploadState(xml_resources, [], IriResolver({"foo_2_id": "foo_2_iri"}), None, UploadConfig(), {}) - con = Mock(spec=ConnectionLive) + con = Mock(spec_set=ConnectionLive) con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}) project_client = ProjectClientStub(con, "1234", None) xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) @@ -131,7 +131,7 @@ def test_two_resources_with_stash() -> None: } stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) upload_state = UploadState(xml_resources, [], IriResolver(), stash, UploadConfig(), {}) - con = Mock(spec=ConnectionLive) + con = Mock(spec_set=ConnectionLive) con.post = Mock( side_effect=[ {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, @@ -185,7 +185,7 @@ def _two_resources_with_stash_interrupted_by_error(err_to_interrupt_with: BaseEx } stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) upload_state = UploadState(xml_resources.copy(), [], IriResolver(), copy(stash), UploadConfig(), {}) - con = Mock(spec=ConnectionLive) + con = Mock(spec_set=ConnectionLive) con.post = Mock(side_effect=[{"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, err_to_interrupt_with]) xmlupload._handle_upload_error = Mock() project_client = ProjectClientStub(con, "1234", None) @@ -221,7 +221,7 @@ def test_two_resources_with_stash_interrupt_after() -> None: stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) upload_config = UploadConfig(interrupt_after=2) upload_state = UploadState(xml_resources.copy(), [], IriResolver(), copy(stash), upload_config, {}) - con = Mock(spec=ConnectionLive) + con = Mock(spec_set=ConnectionLive) con.post = Mock( side_effect=[ {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, From c3ac5d926ee4025152e696d6c9bf86151f98141e Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 29 Apr 2024 10:42:13 +0200 Subject: [PATCH 40/69] refactor tests --- .../test_resource_creation_integration.py | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index ee9af9dbf..1f96c5832 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -60,7 +60,9 @@ def test_one_resource_without_links() -> None: con = Mock(spec_set=ConnectionLive) con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}) project_client = ProjectClientStub(con, "1234", None) + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + assert len(con.post.call_args_list) == 1 match con.post.call_args_list[0].kwargs: case { @@ -95,7 +97,9 @@ def test_one_resource_with_link_to_existing_resource() -> None: con = Mock(spec_set=ConnectionLive) con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}) project_client = ProjectClientStub(con, "1234", None) + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + assert len(con.post.call_args_list) == 1 match con.post.call_args_list[0].kwargs: case { @@ -129,20 +133,23 @@ def test_two_resources_with_stash() -> None: "foo_1_id": [LinkValueStashItem("foo_1_id", "my_onto:foo_1_type", "my_onto:hasCustomLink", "foo_2_id")], "foo_2_id": [LinkValueStashItem("foo_2_id", "my_onto:foo_2_type", "my_onto:hasCustomLink", "foo_1_id")], } - stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) + stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict.copy()), standoff_stash=None) upload_state = UploadState(xml_resources, [], IriResolver(), stash, UploadConfig(), {}) con = Mock(spec_set=ConnectionLive) con.post = Mock( side_effect=[ {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, {"@id": "foo_2_iri", "rdfs:label": "foo_2_label"}, - {}, - {}, + {}, # uploading a stash doesn't rely on a certain response + {}, # uploading a stash doesn't rely on a certain response ] ) project_client = ProjectClientStub(con, "1234", None) + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) - assert len(con.post.call_args_list) == 4 + + num_of_post_requests = len(xml_strings) + len(link_val_stash_dict) # num of resources + num of stashed links + assert len(con.post.call_args_list) == num_of_post_requests match con.post.call_args_list[2].kwargs: case { "route": "/v2/values", @@ -186,10 +193,17 @@ def _two_resources_with_stash_interrupted_by_error(err_to_interrupt_with: BaseEx stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) upload_state = UploadState(xml_resources.copy(), [], IriResolver(), copy(stash), UploadConfig(), {}) con = Mock(spec_set=ConnectionLive) - con.post = Mock(side_effect=[{"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, err_to_interrupt_with]) - xmlupload._handle_upload_error = Mock() + con.post = Mock( + side_effect=[ + {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, + err_to_interrupt_with, + ] + ) project_client = ProjectClientStub(con, "1234", None) + xmlupload._handle_upload_error = Mock() + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + err_msg = ( f"There was a {err_as_str} while trying to create resource 'foo_2_id'.\n" "It is unclear if the resource 'foo_2_id' was created successfully or not.\n" @@ -232,8 +246,8 @@ def test_two_resources_with_stash_interrupt_after() -> None: {"@id": "foo_6_iri", "rdfs:label": "foo_6_label"}, ] ) - xmlupload._handle_upload_error = Mock() project_client = ProjectClientStub(con, "1234", None) + xmlupload._handle_upload_error = Mock() err_msg = "Interrupted: Maximum number of resources was reached (2)" xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) From 88bcf15300c433b25d30cfc6e41f5aba3b39fe8f Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 29 Apr 2024 10:44:20 +0200 Subject: [PATCH 41/69] use numbers in function names --- .../test_resource_creation_integration.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index 1f96c5832..57aa55bb3 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -123,7 +123,7 @@ def test_one_resource_with_link_to_existing_resource() -> None: assert not upload_state.pending_stash -def test_two_resources_with_stash() -> None: +def test_2_resources_with_stash() -> None: xml_strings = [ '', '', @@ -172,15 +172,15 @@ def test_two_resources_with_stash() -> None: assert not upload_state.pending_stash or upload_state.pending_stash.is_empty() -def test_two_resources_with_stash_interrupted_by_timeout() -> None: - _two_resources_with_stash_interrupted_by_error(PermanentTimeOutError(""), "PermanentTimeOutError") +def test_2_resources_with_stash_interrupted_by_timeout() -> None: + _2_resources_with_stash_interrupted_by_error(PermanentTimeOutError(""), "PermanentTimeOutError") -def test_two_resources_with_stash_interrupted_by_keyboard() -> None: - _two_resources_with_stash_interrupted_by_error(KeyboardInterrupt(), "KeyboardInterrupt") +def test_2_resources_with_stash_interrupted_by_keyboard() -> None: + _2_resources_with_stash_interrupted_by_error(KeyboardInterrupt(), "KeyboardInterrupt") -def _two_resources_with_stash_interrupted_by_error(err_to_interrupt_with: BaseException, err_as_str: str) -> None: +def _2_resources_with_stash_interrupted_by_error(err_to_interrupt_with: BaseException, err_as_str: str) -> None: xml_strings = [ '', '', @@ -218,7 +218,7 @@ def _two_resources_with_stash_interrupted_by_error(err_to_interrupt_with: BaseEx xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) -def test_two_resources_with_stash_interrupt_after() -> None: +def test_6_resources_with_stash_and_interrupt_after_2() -> None: xml_strings = [ '', '', From 68ad343e7f7ee1cb7f5443f0885ec6b7ef80a26d Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 29 Apr 2024 10:57:22 +0200 Subject: [PATCH 42/69] add test_5_resources_with_stash_and_interrupt_after_2 --- .../test_resource_creation_integration.py | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index 57aa55bb3..b5ddac125 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -218,6 +218,58 @@ def _2_resources_with_stash_interrupted_by_error(err_to_interrupt_with: BaseExce xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) +def test_5_resources_with_stash_and_interrupt_after_2() -> None: + xml_strings = [ + '', + '', + '', + '', + '', + ] + xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] + link_val_stash_dict = { + "foo_1_id": [LinkValueStashItem("foo_1_id", "my_onto:foo_1_type", "my_onto:hasCustomLink", "foo_2_id")], + "foo_2_id": [LinkValueStashItem("foo_2_id", "my_onto:foo_2_type", "my_onto:hasCustomLink", "foo_1_id")], + } + stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) + upload_config = UploadConfig(interrupt_after=2) + upload_state = UploadState(xml_resources.copy(), [], IriResolver(), copy(stash), upload_config, {}) + con = Mock(spec_set=ConnectionLive) + con.post = Mock( + side_effect=[ + {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, + {"@id": "foo_2_iri", "rdfs:label": "foo_2_label"}, + {"@id": "foo_3_iri", "rdfs:label": "foo_3_label"}, + {"@id": "foo_4_iri", "rdfs:label": "foo_4_label"}, + {"@id": "foo_5_iri", "rdfs:label": "foo_5_label"}, + {}, # uploading a stash doesn't rely on a certain response + {}, # uploading a stash doesn't rely on a certain response + ] + ) + project_client = ProjectClientStub(con, "1234", None) + xmlupload._handle_upload_error = Mock() + err_msg = "Interrupted: Maximum number of resources was reached (2)" + + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + iri_resolver_expected = IriResolver({"foo_1_id": "foo_1_iri", "foo_2_id": "foo_2_iri"}) + upload_state_expected = UploadState(xml_resources[2:], [], iri_resolver_expected, stash, upload_config, {}) + xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) + + xmlupload._handle_upload_error = Mock() + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + iri_resolver_expected.lookup.update({"foo_3_id": "foo_3_iri", "foo_4_id": "foo_4_iri"}) + upload_state_expected = UploadState(xml_resources[4:], [], iri_resolver_expected, stash, upload_config, {}) + xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) + + xmlupload._handle_upload_error = Mock() + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + iri_resolver_expected.lookup.update({"foo_5_id": "foo_5_iri"}) + empty_stash = Stash(standoff_stash=None, link_value_stash=LinkValueStash({})) + upload_state_expected = UploadState([], [], iri_resolver_expected, empty_stash, upload_config, {}) + xmlupload._handle_upload_error.assert_not_called() + assert upload_state == upload_state_expected + + def test_6_resources_with_stash_and_interrupt_after_2() -> None: xml_strings = [ '', From 7e2e2198ae199fd134d95ff3508c37d6972ce94a Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 29 Apr 2024 15:07:41 +0200 Subject: [PATCH 43/69] use deepcopy --- .../xmlupload/test_resource_creation_integration.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index b5ddac125..7bdcc0f20 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -1,4 +1,4 @@ -from copy import copy +from copy import deepcopy from dataclasses import dataclass from unittest.mock import Mock @@ -133,8 +133,8 @@ def test_2_resources_with_stash() -> None: "foo_1_id": [LinkValueStashItem("foo_1_id", "my_onto:foo_1_type", "my_onto:hasCustomLink", "foo_2_id")], "foo_2_id": [LinkValueStashItem("foo_2_id", "my_onto:foo_2_type", "my_onto:hasCustomLink", "foo_1_id")], } - stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict.copy()), standoff_stash=None) - upload_state = UploadState(xml_resources, [], IriResolver(), stash, UploadConfig(), {}) + stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) + upload_state = UploadState(xml_resources, [], IriResolver(), deepcopy(stash), UploadConfig(), {}) con = Mock(spec_set=ConnectionLive) con.post = Mock( side_effect=[ @@ -191,7 +191,7 @@ def _2_resources_with_stash_interrupted_by_error(err_to_interrupt_with: BaseExce "foo_2_id": [LinkValueStashItem("foo_2_id", "my_onto:foo_2_type", "my_onto:hasCustomLink", "foo_1_id")], } stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) - upload_state = UploadState(xml_resources.copy(), [], IriResolver(), copy(stash), UploadConfig(), {}) + upload_state = UploadState(xml_resources.copy(), [], IriResolver(), deepcopy(stash), UploadConfig(), {}) con = Mock(spec_set=ConnectionLive) con.post = Mock( side_effect=[ @@ -233,7 +233,7 @@ def test_5_resources_with_stash_and_interrupt_after_2() -> None: } stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) upload_config = UploadConfig(interrupt_after=2) - upload_state = UploadState(xml_resources.copy(), [], IriResolver(), copy(stash), upload_config, {}) + upload_state = UploadState(xml_resources.copy(), [], IriResolver(), deepcopy(stash), upload_config, {}) con = Mock(spec_set=ConnectionLive) con.post = Mock( side_effect=[ @@ -286,7 +286,7 @@ def test_6_resources_with_stash_and_interrupt_after_2() -> None: } stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) upload_config = UploadConfig(interrupt_after=2) - upload_state = UploadState(xml_resources.copy(), [], IriResolver(), copy(stash), upload_config, {}) + upload_state = UploadState(xml_resources.copy(), [], IriResolver(), deepcopy(stash), upload_config, {}) con = Mock(spec_set=ConnectionLive) con.post = Mock( side_effect=[ From 48bc8137e1a7f4ca3e8a4472a76ec9dc592b5323 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 29 Apr 2024 16:13:27 +0200 Subject: [PATCH 44/69] check num of calls --- .../test_resource_creation_integration.py | 40 +++++++++++-------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index 7bdcc0f20..969ac939b 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -58,12 +58,13 @@ def test_one_resource_without_links() -> None: xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] upload_state = UploadState(xml_resources, [], IriResolver(), None, UploadConfig(), {}) con = Mock(spec_set=ConnectionLive) - con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}) + post_responses = [{"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}] + con.post = Mock(side_effect=post_responses) project_client = ProjectClientStub(con, "1234", None) xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) - assert len(con.post.call_args_list) == 1 + assert len(con.post.call_args_list) == len(post_responses) match con.post.call_args_list[0].kwargs: case { "route": "/v2/resources", @@ -95,12 +96,13 @@ def test_one_resource_with_link_to_existing_resource() -> None: xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] upload_state = UploadState(xml_resources, [], IriResolver({"foo_2_id": "foo_2_iri"}), None, UploadConfig(), {}) con = Mock(spec_set=ConnectionLive) - con.post = Mock(return_value={"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}) + post_responses = [{"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}] + con.post = Mock(side_effect=post_responses) project_client = ProjectClientStub(con, "1234", None) xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) - assert len(con.post.call_args_list) == 1 + assert len(con.post.call_args_list) == len(post_responses) match con.post.call_args_list[0].kwargs: case { "route": "/v2/resources", @@ -136,20 +138,18 @@ def test_2_resources_with_stash() -> None: stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) upload_state = UploadState(xml_resources, [], IriResolver(), deepcopy(stash), UploadConfig(), {}) con = Mock(spec_set=ConnectionLive) - con.post = Mock( - side_effect=[ + post_responses = [ {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, {"@id": "foo_2_iri", "rdfs:label": "foo_2_label"}, {}, # uploading a stash doesn't rely on a certain response {}, # uploading a stash doesn't rely on a certain response ] - ) + con.post = Mock(side_effect=post_responses) project_client = ProjectClientStub(con, "1234", None) xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) - num_of_post_requests = len(xml_strings) + len(link_val_stash_dict) # num of resources + num of stashed links - assert len(con.post.call_args_list) == num_of_post_requests + assert len(con.post.call_args_list) == len(post_responses) match con.post.call_args_list[2].kwargs: case { "route": "/v2/values", @@ -193,17 +193,17 @@ def _2_resources_with_stash_interrupted_by_error(err_to_interrupt_with: BaseExce stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) upload_state = UploadState(xml_resources.copy(), [], IriResolver(), deepcopy(stash), UploadConfig(), {}) con = Mock(spec_set=ConnectionLive) - con.post = Mock( - side_effect=[ + post_responses = [ {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, err_to_interrupt_with, ] - ) + con.post = Mock(side_effect=post_responses) project_client = ProjectClientStub(con, "1234", None) xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + assert len(con.post.call_args_list) == len(post_responses) err_msg = ( f"There was a {err_as_str} while trying to create resource 'foo_2_id'.\n" "It is unclear if the resource 'foo_2_id' was created successfully or not.\n" @@ -232,11 +232,11 @@ def test_5_resources_with_stash_and_interrupt_after_2() -> None: "foo_2_id": [LinkValueStashItem("foo_2_id", "my_onto:foo_2_type", "my_onto:hasCustomLink", "foo_1_id")], } stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) - upload_config = UploadConfig(interrupt_after=2) + interrupt_after = 2 + upload_config = UploadConfig(interrupt_after=interrupt_after) upload_state = UploadState(xml_resources.copy(), [], IriResolver(), deepcopy(stash), upload_config, {}) con = Mock(spec_set=ConnectionLive) - con.post = Mock( - side_effect=[ + post_responses = [ {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, {"@id": "foo_2_iri", "rdfs:label": "foo_2_label"}, {"@id": "foo_3_iri", "rdfs:label": "foo_3_label"}, @@ -245,7 +245,7 @@ def test_5_resources_with_stash_and_interrupt_after_2() -> None: {}, # uploading a stash doesn't rely on a certain response {}, # uploading a stash doesn't rely on a certain response ] - ) + con.post = Mock(side_effect=post_responses) project_client = ProjectClientStub(con, "1234", None) xmlupload._handle_upload_error = Mock() err_msg = "Interrupted: Maximum number of resources was reached (2)" @@ -254,12 +254,14 @@ def test_5_resources_with_stash_and_interrupt_after_2() -> None: iri_resolver_expected = IriResolver({"foo_1_id": "foo_1_iri", "foo_2_id": "foo_2_iri"}) upload_state_expected = UploadState(xml_resources[2:], [], iri_resolver_expected, stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) + assert len(con.post.call_args_list) == interrupt_after xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) iri_resolver_expected.lookup.update({"foo_3_id": "foo_3_iri", "foo_4_id": "foo_4_iri"}) upload_state_expected = UploadState(xml_resources[4:], [], iri_resolver_expected, stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) + assert len(con.post.call_args_list) == interrupt_after xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) @@ -268,6 +270,7 @@ def test_5_resources_with_stash_and_interrupt_after_2() -> None: upload_state_expected = UploadState([], [], iri_resolver_expected, empty_stash, upload_config, {}) xmlupload._handle_upload_error.assert_not_called() assert upload_state == upload_state_expected + assert len(con.post.call_args_list) == 3 # 1 resource + 2 stashed links def test_6_resources_with_stash_and_interrupt_after_2() -> None: @@ -285,7 +288,8 @@ def test_6_resources_with_stash_and_interrupt_after_2() -> None: "foo_2_id": [LinkValueStashItem("foo_2_id", "my_onto:foo_2_type", "my_onto:hasCustomLink", "foo_1_id")], } stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) - upload_config = UploadConfig(interrupt_after=2) + interrupt_after = 2 + upload_config = UploadConfig(interrupt_after=interrupt_after) upload_state = UploadState(xml_resources.copy(), [], IriResolver(), deepcopy(stash), upload_config, {}) con = Mock(spec_set=ConnectionLive) con.post = Mock( @@ -306,12 +310,14 @@ def test_6_resources_with_stash_and_interrupt_after_2() -> None: iri_resolver_expected = IriResolver({"foo_1_id": "foo_1_iri", "foo_2_id": "foo_2_iri"}) upload_state_expected = UploadState(xml_resources[2:], [], iri_resolver_expected, stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) + assert len(con.post.call_args_list) == interrupt_after xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) iri_resolver_expected.lookup.update({"foo_3_id": "foo_3_iri", "foo_4_id": "foo_4_iri"}) upload_state_expected = UploadState(xml_resources[4:], [], iri_resolver_expected, stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) + assert len(con.post.call_args_list) == interrupt_after xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) From c414d000a0d4a4af01b131109f0a781275d85d06 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 29 Apr 2024 16:26:35 +0200 Subject: [PATCH 45/69] fix last test --- .../test_resource_creation_integration.py | 86 +++++++++++-------- 1 file changed, 49 insertions(+), 37 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index 969ac939b..db24a7b61 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -139,11 +139,11 @@ def test_2_resources_with_stash() -> None: upload_state = UploadState(xml_resources, [], IriResolver(), deepcopy(stash), UploadConfig(), {}) con = Mock(spec_set=ConnectionLive) post_responses = [ - {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, - {"@id": "foo_2_iri", "rdfs:label": "foo_2_label"}, - {}, # uploading a stash doesn't rely on a certain response - {}, # uploading a stash doesn't rely on a certain response - ] + {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, + {"@id": "foo_2_iri", "rdfs:label": "foo_2_label"}, + {}, # uploading a stash doesn't rely on a certain response + {}, # uploading a stash doesn't rely on a certain response + ] con.post = Mock(side_effect=post_responses) project_client = ProjectClientStub(con, "1234", None) @@ -194,9 +194,9 @@ def _2_resources_with_stash_interrupted_by_error(err_to_interrupt_with: BaseExce upload_state = UploadState(xml_resources.copy(), [], IriResolver(), deepcopy(stash), UploadConfig(), {}) con = Mock(spec_set=ConnectionLive) post_responses = [ - {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, - err_to_interrupt_with, - ] + {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, + err_to_interrupt_with, + ] con.post = Mock(side_effect=post_responses) project_client = ProjectClientStub(con, "1234", None) xmlupload._handle_upload_error = Mock() @@ -232,19 +232,18 @@ def test_5_resources_with_stash_and_interrupt_after_2() -> None: "foo_2_id": [LinkValueStashItem("foo_2_id", "my_onto:foo_2_type", "my_onto:hasCustomLink", "foo_1_id")], } stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) - interrupt_after = 2 - upload_config = UploadConfig(interrupt_after=interrupt_after) + upload_config = UploadConfig(interrupt_after=2) upload_state = UploadState(xml_resources.copy(), [], IriResolver(), deepcopy(stash), upload_config, {}) con = Mock(spec_set=ConnectionLive) post_responses = [ - {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, - {"@id": "foo_2_iri", "rdfs:label": "foo_2_label"}, - {"@id": "foo_3_iri", "rdfs:label": "foo_3_label"}, - {"@id": "foo_4_iri", "rdfs:label": "foo_4_label"}, - {"@id": "foo_5_iri", "rdfs:label": "foo_5_label"}, - {}, # uploading a stash doesn't rely on a certain response - {}, # uploading a stash doesn't rely on a certain response - ] + {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, + {"@id": "foo_2_iri", "rdfs:label": "foo_2_label"}, + {"@id": "foo_3_iri", "rdfs:label": "foo_3_label"}, + {"@id": "foo_4_iri", "rdfs:label": "foo_4_label"}, + {"@id": "foo_5_iri", "rdfs:label": "foo_5_label"}, + {}, # uploading a stash doesn't rely on a certain response + {}, # uploading a stash doesn't rely on a certain response + ] con.post = Mock(side_effect=post_responses) project_client = ProjectClientStub(con, "1234", None) xmlupload._handle_upload_error = Mock() @@ -254,14 +253,14 @@ def test_5_resources_with_stash_and_interrupt_after_2() -> None: iri_resolver_expected = IriResolver({"foo_1_id": "foo_1_iri", "foo_2_id": "foo_2_iri"}) upload_state_expected = UploadState(xml_resources[2:], [], iri_resolver_expected, stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) - assert len(con.post.call_args_list) == interrupt_after + assert len(con.post.call_args_list) == 2 xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) iri_resolver_expected.lookup.update({"foo_3_id": "foo_3_iri", "foo_4_id": "foo_4_iri"}) upload_state_expected = UploadState(xml_resources[4:], [], iri_resolver_expected, stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) - assert len(con.post.call_args_list) == interrupt_after + assert len(con.post.call_args_list) == 4 xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) @@ -270,7 +269,7 @@ def test_5_resources_with_stash_and_interrupt_after_2() -> None: upload_state_expected = UploadState([], [], iri_resolver_expected, empty_stash, upload_config, {}) xmlupload._handle_upload_error.assert_not_called() assert upload_state == upload_state_expected - assert len(con.post.call_args_list) == 3 # 1 resource + 2 stashed links + assert len(con.post.call_args_list) == 7 def test_6_resources_with_stash_and_interrupt_after_2() -> None: @@ -288,20 +287,20 @@ def test_6_resources_with_stash_and_interrupt_after_2() -> None: "foo_2_id": [LinkValueStashItem("foo_2_id", "my_onto:foo_2_type", "my_onto:hasCustomLink", "foo_1_id")], } stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) - interrupt_after = 2 - upload_config = UploadConfig(interrupt_after=interrupt_after) + upload_config = UploadConfig(interrupt_after=2) upload_state = UploadState(xml_resources.copy(), [], IriResolver(), deepcopy(stash), upload_config, {}) con = Mock(spec_set=ConnectionLive) - con.post = Mock( - side_effect=[ - {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, - {"@id": "foo_2_iri", "rdfs:label": "foo_2_label"}, - {"@id": "foo_3_iri", "rdfs:label": "foo_3_label"}, - {"@id": "foo_4_iri", "rdfs:label": "foo_4_label"}, - {"@id": "foo_5_iri", "rdfs:label": "foo_5_label"}, - {"@id": "foo_6_iri", "rdfs:label": "foo_6_label"}, - ] - ) + post_responses = [ + {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, + {"@id": "foo_2_iri", "rdfs:label": "foo_2_label"}, + {"@id": "foo_3_iri", "rdfs:label": "foo_3_label"}, + {"@id": "foo_4_iri", "rdfs:label": "foo_4_label"}, + {"@id": "foo_5_iri", "rdfs:label": "foo_5_label"}, + {"@id": "foo_6_iri", "rdfs:label": "foo_6_label"}, + {}, # uploading a stash doesn't rely on a certain response + {}, # uploading a stash doesn't rely on a certain response + ] + con.post = Mock(side_effect=post_responses) project_client = ProjectClientStub(con, "1234", None) xmlupload._handle_upload_error = Mock() err_msg = "Interrupted: Maximum number of resources was reached (2)" @@ -310,17 +309,30 @@ def test_6_resources_with_stash_and_interrupt_after_2() -> None: iri_resolver_expected = IriResolver({"foo_1_id": "foo_1_iri", "foo_2_id": "foo_2_iri"}) upload_state_expected = UploadState(xml_resources[2:], [], iri_resolver_expected, stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) - assert len(con.post.call_args_list) == interrupt_after + assert len(con.post.call_args_list) == 2 xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) iri_resolver_expected.lookup.update({"foo_3_id": "foo_3_iri", "foo_4_id": "foo_4_iri"}) upload_state_expected = UploadState(xml_resources[4:], [], iri_resolver_expected, stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) - assert len(con.post.call_args_list) == interrupt_after + assert len(con.post.call_args_list) == 4 xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) iri_resolver_expected.lookup.update({"foo_5_id": "foo_5_iri", "foo_6_id": "foo_6_iri"}) - upload_state_expected = UploadState([], [], iri_resolver_expected, stash, upload_config, {}) - xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) + upload_state_expected_before_stash_upload = UploadState([], [], iri_resolver_expected, stash, upload_config, {}) + xmlupload._handle_upload_error.assert_called_once_with( + XmlUploadInterruptedError(err_msg), upload_state_expected_before_stash_upload + ) + assert len(con.post.call_args_list) == 6 + + xmlupload._handle_upload_error = Mock() + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + empty_stash = Stash(standoff_stash=None, link_value_stash=LinkValueStash({})) + upload_state_expected_after_stash_upload = UploadState( + [], [], iri_resolver_expected, empty_stash, upload_config, {} + ) + xmlupload._handle_upload_error.assert_not_called() + assert upload_state == upload_state_expected_after_stash_upload + assert len(con.post.call_args_list) == 8 From 47efc0fe16d8e312f487b66c08483cc07fcc8f07 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 29 Apr 2024 16:53:56 +0200 Subject: [PATCH 46/69] reraise keyboardinterrupt after tidy up --- src/dsp_tools/commands/xmlupload/xmlupload.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index 7e410e4fb..541811854 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -380,11 +380,12 @@ def _upload_one_resource( _tidy_up_resource_creation_idempotent( upload_state, iri, label, resource, creation_attempts_of_this_round + 1, total_num_of_resources ) - except KeyboardInterrupt: + except KeyboardInterrupt as err: warnings.warn(GeneralDspToolsWarning("KeyboardInterrupt: Tidying up, then exit...")) _tidy_up_resource_creation_idempotent( upload_state, iri, label, resource, creation_attempts_of_this_round + 1, total_num_of_resources ) + raise err from None _interrupt_if_indicated(upload_state, total_num_of_resources, creation_attempts_of_this_round) From 3e5945af7987272917f500f08a0a0898125b597b Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 29 Apr 2024 18:41:29 +0200 Subject: [PATCH 47/69] test post calls separately --- .../test_resource_creation_integration.py | 46 ++++++++++++++++--- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index db24a7b61..ccf16bd66 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -253,14 +253,12 @@ def test_5_resources_with_stash_and_interrupt_after_2() -> None: iri_resolver_expected = IriResolver({"foo_1_id": "foo_1_iri", "foo_2_id": "foo_2_iri"}) upload_state_expected = UploadState(xml_resources[2:], [], iri_resolver_expected, stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) - assert len(con.post.call_args_list) == 2 xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) iri_resolver_expected.lookup.update({"foo_3_id": "foo_3_iri", "foo_4_id": "foo_4_iri"}) upload_state_expected = UploadState(xml_resources[4:], [], iri_resolver_expected, stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) - assert len(con.post.call_args_list) == 4 xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) @@ -269,7 +267,6 @@ def test_5_resources_with_stash_and_interrupt_after_2() -> None: upload_state_expected = UploadState([], [], iri_resolver_expected, empty_stash, upload_config, {}) xmlupload._handle_upload_error.assert_not_called() assert upload_state == upload_state_expected - assert len(con.post.call_args_list) == 7 def test_6_resources_with_stash_and_interrupt_after_2() -> None: @@ -309,14 +306,12 @@ def test_6_resources_with_stash_and_interrupt_after_2() -> None: iri_resolver_expected = IriResolver({"foo_1_id": "foo_1_iri", "foo_2_id": "foo_2_iri"}) upload_state_expected = UploadState(xml_resources[2:], [], iri_resolver_expected, stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) - assert len(con.post.call_args_list) == 2 xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) iri_resolver_expected.lookup.update({"foo_3_id": "foo_3_iri", "foo_4_id": "foo_4_iri"}) upload_state_expected = UploadState(xml_resources[4:], [], iri_resolver_expected, stash, upload_config, {}) xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) - assert len(con.post.call_args_list) == 4 xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) @@ -325,7 +320,6 @@ def test_6_resources_with_stash_and_interrupt_after_2() -> None: xmlupload._handle_upload_error.assert_called_once_with( XmlUploadInterruptedError(err_msg), upload_state_expected_before_stash_upload ) - assert len(con.post.call_args_list) == 6 xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) @@ -335,4 +329,42 @@ def test_6_resources_with_stash_and_interrupt_after_2() -> None: ) xmlupload._handle_upload_error.assert_not_called() assert upload_state == upload_state_expected_after_stash_upload - assert len(con.post.call_args_list) == 8 + + +def test_post_calls() -> None: + xml_strings = [ + '', + '', + '', + '', + '', + '', + ] + xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] + link_val_stash_dict = { + "foo_1_id": [LinkValueStashItem("foo_1_id", "my_onto:foo_1_type", "my_onto:hasCustomLink", "foo_2_id")], + "foo_2_id": [LinkValueStashItem("foo_2_id", "my_onto:foo_2_type", "my_onto:hasCustomLink", "foo_1_id")], + } + stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) + upload_config = UploadConfig(interrupt_after=2) + upload_state = UploadState(xml_resources.copy(), [], IriResolver(), deepcopy(stash), upload_config, {}) + con = Mock(spec_set=ConnectionLive) + post_responses = [ + {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, + {"@id": "foo_2_iri", "rdfs:label": "foo_2_label"}, + {"@id": "foo_3_iri", "rdfs:label": "foo_3_label"}, + {"@id": "foo_4_iri", "rdfs:label": "foo_4_label"}, + {"@id": "foo_5_iri", "rdfs:label": "foo_5_label"}, + {"@id": "foo_6_iri", "rdfs:label": "foo_6_label"}, + {}, # uploading a stash doesn't rely on a certain response + {}, # uploading a stash doesn't rely on a certain response + ] + con.post = Mock(side_effect=post_responses) + project_client = ProjectClientStub(con, "1234", None) + xmlupload._handle_upload_error = Mock() + + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + assert len(con.post.call_args_list) == len(post_responses) From e17666ae522e521ccd5dac8520d6cd7c19da6319 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 29 Apr 2024 22:07:24 +0200 Subject: [PATCH 48/69] shorter variable names --- .../xmlupload/test_resource_creation_integration.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index ccf16bd66..4b0cea1e1 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -316,19 +316,15 @@ def test_6_resources_with_stash_and_interrupt_after_2() -> None: xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) iri_resolver_expected.lookup.update({"foo_5_id": "foo_5_iri", "foo_6_id": "foo_6_iri"}) - upload_state_expected_before_stash_upload = UploadState([], [], iri_resolver_expected, stash, upload_config, {}) - xmlupload._handle_upload_error.assert_called_once_with( - XmlUploadInterruptedError(err_msg), upload_state_expected_before_stash_upload - ) + upload_state_expected = UploadState([], [], iri_resolver_expected, stash, upload_config, {}) + xmlupload._handle_upload_error.assert_called_once_with(XmlUploadInterruptedError(err_msg), upload_state_expected) xmlupload._handle_upload_error = Mock() xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) empty_stash = Stash(standoff_stash=None, link_value_stash=LinkValueStash({})) - upload_state_expected_after_stash_upload = UploadState( - [], [], iri_resolver_expected, empty_stash, upload_config, {} - ) + upload_state_expected = UploadState([], [], iri_resolver_expected, empty_stash, upload_config, {}) xmlupload._handle_upload_error.assert_not_called() - assert upload_state == upload_state_expected_after_stash_upload + assert upload_state == upload_state_expected def test_post_calls() -> None: From c1295d2338e6ac6fc4c830f46e3d01c000cb0074 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Mon, 29 Apr 2024 22:42:19 +0200 Subject: [PATCH 49/69] add a test for logging --- .../test_resource_creation_integration.py | 52 ++++++++++++++++++- test/integration/conftest.py | 17 ++++++ 2 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 test/integration/conftest.py diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index 4b0cea1e1..3985eb328 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -85,6 +85,56 @@ def test_one_resource_without_links() -> None: assert not upload_state.pending_stash +def test_logging(caplog: pytest.LogCaptureFixture) -> None: + caplog.set_level("INFO") + xml_strings = [ + '', + '', + '', + '', + '', + ] + xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] + link_val_stash_dict = { + "foo_1_id": [LinkValueStashItem("foo_1_id", "my_onto:foo_1_type", "my_onto:hasCustomLink", "foo_2_id")], + "foo_2_id": [LinkValueStashItem("foo_2_id", "my_onto:foo_2_type", "my_onto:hasCustomLink", "foo_1_id")], + } + stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) + upload_config = UploadConfig(interrupt_after=2) + upload_state = UploadState(xml_resources.copy(), [], IriResolver(), deepcopy(stash), upload_config, {}) + con = Mock(spec_set=ConnectionLive) + post_responses = [ + {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, + {"@id": "foo_2_iri", "rdfs:label": "foo_2_label"}, + {"@id": "foo_3_iri", "rdfs:label": "foo_3_label"}, + {"@id": "foo_4_iri", "rdfs:label": "foo_4_label"}, + {"@id": "foo_5_iri", "rdfs:label": "foo_5_label"}, + {}, # uploading a stash doesn't rely on a certain response + {}, # uploading a stash doesn't rely on a certain response + ] + con.post = Mock(side_effect=post_responses) + project_client = ProjectClientStub(con, "1234", None) + xmlupload._handle_upload_error = Mock() + + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + assert caplog.records[1].message == "Created resource 1/5: 'foo_1_label' (ID: 'foo_1_id', IRI: 'foo_1_iri')" + assert caplog.records[3].message == "Created resource 2/5: 'foo_2_label' (ID: 'foo_2_id', IRI: 'foo_2_iri')" + caplog.clear() + + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + assert caplog.records[1].message == "Created resource 3/5: 'foo_3_label' (ID: 'foo_3_id', IRI: 'foo_3_iri')" + assert caplog.records[3].message == "Created resource 4/5: 'foo_4_label' (ID: 'foo_4_id', IRI: 'foo_4_iri')" + caplog.clear() + + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + assert caplog.records[1].message == "Created resource 5/5: 'foo_5_label' (ID: 'foo_5_id', IRI: 'foo_5_iri')" + assert caplog.records[3].message == "Upload resptrs of resource 'foo_1_id'..." + assert caplog.records[4].message == 'Successfully uploaded resptr links of "my_onto:hasCustomLink"' + assert caplog.records[5].message == "Upload resptrs of resource 'foo_2_id'..." + assert caplog.records[6].message == 'Successfully uploaded resptr links of "my_onto:hasCustomLink"' + caplog.clear() + + def test_one_resource_with_link_to_existing_resource() -> None: xml_strings = [ """ @@ -327,7 +377,7 @@ def test_6_resources_with_stash_and_interrupt_after_2() -> None: assert upload_state == upload_state_expected -def test_post_calls() -> None: +def test_post_requests() -> None: xml_strings = [ '', '', diff --git a/test/integration/conftest.py b/test/integration/conftest.py new file mode 100644 index 000000000..1eae1e4e1 --- /dev/null +++ b/test/integration/conftest.py @@ -0,0 +1,17 @@ +import logging +from typing import Iterator + +import pytest +from _pytest.logging import caplog as _caplog # noqa: F401 (imported but unused) +from loguru import logger + + +@pytest.fixture() +def caplog(_caplog: pytest.LogCaptureFixture) -> Iterator[pytest.LogCaptureFixture]: # noqa: F811 (redefinition) + class PropagateHandler(logging.Handler): + def emit(self, record: logging.LogRecord) -> None: + logging.getLogger(record.name).handle(record) + + handler_id = logger.add(PropagateHandler(), format="{message}") + yield _caplog + logger.remove(handler_id) From 8fb7e45128008c7c9640f7e156a4425f2eb40f2b Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 07:42:54 +0200 Subject: [PATCH 50/69] don't return label --- .../commands/xmlupload/resource_create_client.py | 7 +++---- src/dsp_tools/commands/xmlupload/xmlupload.py | 15 +++++++-------- .../xmlupload/test_resource_creation_unit.py | 4 ++-- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/resource_create_client.py b/src/dsp_tools/commands/xmlupload/resource_create_client.py index 3a335bc7d..01c337248 100644 --- a/src/dsp_tools/commands/xmlupload/resource_create_client.py +++ b/src/dsp_tools/commands/xmlupload/resource_create_client.py @@ -3,6 +3,7 @@ from pathlib import Path from typing import Any from typing import assert_never +from typing import cast from loguru import logger @@ -37,7 +38,7 @@ def create_resource( self, resource: XMLResource, bitstream_information: BitstreamInfo | None, - ) -> tuple[str, str]: + ) -> str: """Creates a resource on the DSP server.""" logger.info( f"Attempting to create resource {resource.res_id} (label: {resource.label}, iri: {resource.iri})..." @@ -45,9 +46,7 @@ def create_resource( resource_dict = self._make_resource_with_values(resource, bitstream_information) headers = {"X-Asset-Ingested": "true"} if self.media_previously_ingested else None res = self.con.post(route="/v2/resources", data=resource_dict, headers=headers) - iri = res["@id"] - label = res["rdfs:label"] - return iri, label + return cast(str, res["@id"]) def _make_resource_with_values( self, diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index 541811854..3cbf72f22 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -362,7 +362,7 @@ def _upload_one_resource( return try: - iri, label = _create_resource(resource, media_info, resource_create_client) + iri = _create_resource(resource, media_info, resource_create_client) except (PermanentTimeOutError, KeyboardInterrupt) as err: warnings.warn(GeneralDspToolsWarning(f"{type(err).__name__}: Tidying up, then exit...")) msg = ( @@ -378,12 +378,12 @@ def _upload_one_resource( try: _tidy_up_resource_creation_idempotent( - upload_state, iri, label, resource, creation_attempts_of_this_round + 1, total_num_of_resources + upload_state, iri, resource, creation_attempts_of_this_round + 1, total_num_of_resources ) except KeyboardInterrupt as err: warnings.warn(GeneralDspToolsWarning("KeyboardInterrupt: Tidying up, then exit...")) _tidy_up_resource_creation_idempotent( - upload_state, iri, label, resource, creation_attempts_of_this_round + 1, total_num_of_resources + upload_state, iri, resource, creation_attempts_of_this_round + 1, total_num_of_resources ) raise err from None @@ -404,15 +404,14 @@ def _interrupt_if_indicated( def _tidy_up_resource_creation_idempotent( upload_state: UploadState, iri: str | None, - label: str | None, resource: XMLResource, current_res: int, total_res: int, ) -> None: - if iri and label: + if iri: # resource creation succeeded: update the iri_resolver upload_state.iri_resolver.lookup[resource.res_id] = iri - msg = f"Created resource {current_res}/{total_res}: '{label}' (ID: '{resource.res_id}', IRI: '{iri}')" + msg = f"Created resource {current_res}/{total_res}: '{resource.label}' (ID: '{resource.res_id}', IRI: '{iri}')" print(f"{datetime.now()}: {msg}") logger.info(msg) else: # noqa: PLR5501 @@ -428,7 +427,7 @@ def _create_resource( resource: XMLResource, bitstream_information: BitstreamInfo | None, resource_create_client: ResourceCreateClient, -) -> tuple[str, str] | tuple[None, None]: +) -> str | None: try: return resource_create_client.create_resource(resource, bitstream_information) except PermanentTimeOutError as err: @@ -447,7 +446,7 @@ def _create_resource( f"Property details:\n" + "\n".join([str(vars(prop)) for prop in resource.properties]) ) logger.exception(log_msg) - return None, None + return None def _handle_upload_error(err: BaseException, upload_state: UploadState) -> None: diff --git a/test/unittests/commands/xmlupload/test_resource_creation_unit.py b/test/unittests/commands/xmlupload/test_resource_creation_unit.py index 16e9a148c..cc8a9ca69 100644 --- a/test/unittests/commands/xmlupload/test_resource_creation_unit.py +++ b/test/unittests/commands/xmlupload/test_resource_creation_unit.py @@ -23,7 +23,7 @@ def test_idempotency_on_success() -> None: xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] upload_state = UploadState(xml_resources.copy(), [], IriResolver(), None, UploadConfig(), {}) for _ in range(3): - _tidy_up_resource_creation_idempotent(upload_state, "foo_1_iri", "foo_1_label", xml_resources[0], 1, 2) + _tidy_up_resource_creation_idempotent(upload_state, "foo_1_iri", xml_resources[0], 1, 2) assert upload_state.pending_resources == xml_resources[1:] assert upload_state.failed_uploads == [] assert upload_state.iri_resolver.lookup == {"foo_1_id": "foo_1_iri"} @@ -46,7 +46,7 @@ def test_idempotency_on_failure() -> None: xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] upload_state = UploadState(xml_resources.copy(), [], IriResolver(), None, UploadConfig(), {}) for _ in range(3): - _tidy_up_resource_creation_idempotent(upload_state, None, None, xml_resources[0], 1, 2) + _tidy_up_resource_creation_idempotent(upload_state, None, xml_resources[0], 1, 2) assert upload_state.pending_resources == xml_resources[1:] assert upload_state.failed_uploads == ["foo_1_id"] assert upload_state.iri_resolver.lookup == {} From b8ca72c0bbe5a6bc132828e03efa0033dc295ae3 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 07:52:44 +0200 Subject: [PATCH 51/69] compute numbers at lowest level --- src/dsp_tools/commands/xmlupload/xmlupload.py | 29 +++++++------------ .../xmlupload/test_resource_creation_unit.py | 4 +-- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index 3cbf72f22..47499736b 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -328,10 +328,6 @@ def _upload_resources( listnode_lookup=listnode_lookup, media_previously_ingested=upload_state.config.media_previously_uploaded, ) - previous_successful = len(upload_state.iri_resolver.lookup) - previous_failed = len(upload_state.failed_uploads) - upcoming = len(upload_state.pending_resources) - total_num_of_resources = previous_successful + previous_failed + upcoming for creation_attempts_of_this_round, resource in enumerate(upload_state.pending_resources.copy()): _upload_one_resource( @@ -340,7 +336,6 @@ def _upload_resources( imgdir=imgdir, sipi_server=sipi_server, resource_create_client=resource_create_client, - total_num_of_resources=total_num_of_resources, creation_attempts_of_this_round=creation_attempts_of_this_round, ) @@ -351,7 +346,6 @@ def _upload_one_resource( imgdir: str, sipi_server: Sipi, resource_create_client: ResourceCreateClient, - total_num_of_resources: int, creation_attempts_of_this_round: int, ) -> None: success, media_info = handle_media_info( @@ -377,24 +371,18 @@ def _upload_one_resource( raise XmlUploadInterruptedError(msg) from None try: - _tidy_up_resource_creation_idempotent( - upload_state, iri, resource, creation_attempts_of_this_round + 1, total_num_of_resources - ) + _tidy_up_resource_creation_idempotent(upload_state, iri, resource) except KeyboardInterrupt as err: warnings.warn(GeneralDspToolsWarning("KeyboardInterrupt: Tidying up, then exit...")) - _tidy_up_resource_creation_idempotent( - upload_state, iri, resource, creation_attempts_of_this_round + 1, total_num_of_resources - ) + _tidy_up_resource_creation_idempotent(upload_state, iri, resource) raise err from None - _interrupt_if_indicated(upload_state, total_num_of_resources, creation_attempts_of_this_round) + _interrupt_if_indicated(upload_state, creation_attempts_of_this_round) -def _interrupt_if_indicated( - upload_state: UploadState, total_num_of_resources: int, creation_attempts_of_this_round: int -) -> None: +def _interrupt_if_indicated(upload_state: UploadState, creation_attempts_of_this_round: int) -> None: # if the interrupt_after value is not set, the upload will not be interrupted - interrupt_after = upload_state.config.interrupt_after or total_num_of_resources + 1 + interrupt_after = upload_state.config.interrupt_after or 999_999_999 if creation_attempts_of_this_round + 1 >= interrupt_after: raise XmlUploadInterruptedError( f"Interrupted: Maximum number of resources was reached ({upload_state.config.interrupt_after})" @@ -405,9 +393,12 @@ def _tidy_up_resource_creation_idempotent( upload_state: UploadState, iri: str | None, resource: XMLResource, - current_res: int, - total_res: int, ) -> None: + previous_successful = len(upload_state.iri_resolver.lookup) + previous_failed = len(upload_state.failed_uploads) + upcoming = len(upload_state.pending_resources) + current_res = previous_successful + previous_failed + 1 + total_res = previous_successful + previous_failed + upcoming if iri: # resource creation succeeded: update the iri_resolver upload_state.iri_resolver.lookup[resource.res_id] = iri diff --git a/test/unittests/commands/xmlupload/test_resource_creation_unit.py b/test/unittests/commands/xmlupload/test_resource_creation_unit.py index cc8a9ca69..a11b4d362 100644 --- a/test/unittests/commands/xmlupload/test_resource_creation_unit.py +++ b/test/unittests/commands/xmlupload/test_resource_creation_unit.py @@ -23,7 +23,7 @@ def test_idempotency_on_success() -> None: xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] upload_state = UploadState(xml_resources.copy(), [], IriResolver(), None, UploadConfig(), {}) for _ in range(3): - _tidy_up_resource_creation_idempotent(upload_state, "foo_1_iri", xml_resources[0], 1, 2) + _tidy_up_resource_creation_idempotent(upload_state, "foo_1_iri", xml_resources[0]) assert upload_state.pending_resources == xml_resources[1:] assert upload_state.failed_uploads == [] assert upload_state.iri_resolver.lookup == {"foo_1_id": "foo_1_iri"} @@ -46,7 +46,7 @@ def test_idempotency_on_failure() -> None: xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] upload_state = UploadState(xml_resources.copy(), [], IriResolver(), None, UploadConfig(), {}) for _ in range(3): - _tidy_up_resource_creation_idempotent(upload_state, None, xml_resources[0], 1, 2) + _tidy_up_resource_creation_idempotent(upload_state, None, xml_resources[0]) assert upload_state.pending_resources == xml_resources[1:] assert upload_state.failed_uploads == ["foo_1_id"] assert upload_state.iri_resolver.lookup == {} From f91122e0a00f52bbf25198f750cc4a24820fbe9e Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 07:59:36 +0200 Subject: [PATCH 52/69] one single conftest.py --- test/{integration => }/conftest.py | 11 +++++++++++ test/e2e/conftest.py | 10 ---------- 2 files changed, 11 insertions(+), 10 deletions(-) rename test/{integration => }/conftest.py (61%) delete mode 100644 test/e2e/conftest.py diff --git a/test/integration/conftest.py b/test/conftest.py similarity index 61% rename from test/integration/conftest.py rename to test/conftest.py index 1eae1e4e1..65c1050d4 100644 --- a/test/integration/conftest.py +++ b/test/conftest.py @@ -5,6 +5,8 @@ from _pytest.logging import caplog as _caplog # noqa: F401 (imported but unused) from loguru import logger +from dsp_tools.utils.logger_config import logger_config + @pytest.fixture() def caplog(_caplog: pytest.LogCaptureFixture) -> Iterator[pytest.LogCaptureFixture]: # noqa: F811 (redefinition) @@ -15,3 +17,12 @@ def emit(self, record: logging.LogRecord) -> None: handler_id = logger.add(PropagateHandler(), format="{message}") yield _caplog logger.remove(handler_id) + + +def pytest_sessionstart() -> None: + """ + Called after the Session object has been created + and before performing collection and entering the run test loop. + See https://docs.pytest.org/en/latest/reference/reference.html#pytest.hookspec.pytest_sessionstart. + """ + logger_config() diff --git a/test/e2e/conftest.py b/test/e2e/conftest.py deleted file mode 100644 index e28dc796b..000000000 --- a/test/e2e/conftest.py +++ /dev/null @@ -1,10 +0,0 @@ -from dsp_tools.utils.logger_config import logger_config - - -def pytest_sessionstart() -> None: - """ - Called after the Session object has been created - and before performing collection and entering the run test loop. - See https://docs.pytest.org/en/latest/reference/reference.html#pytest.hookspec.pytest_sessionstart. - """ - logger_config() From 193182373788e3cd2c5e6cc408fd646e70b3596f Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 09:26:52 +0200 Subject: [PATCH 53/69] fix logging test --- .../xmlupload/test_resource_creation_integration.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index 3985eb328..1057075b9 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -86,7 +86,7 @@ def test_one_resource_without_links() -> None: def test_logging(caplog: pytest.LogCaptureFixture) -> None: - caplog.set_level("INFO") + caplog.set_level("DEBUG") xml_strings = [ '', '', @@ -128,10 +128,10 @@ def test_logging(caplog: pytest.LogCaptureFixture) -> None: xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) assert caplog.records[1].message == "Created resource 5/5: 'foo_5_label' (ID: 'foo_5_id', IRI: 'foo_5_iri')" - assert caplog.records[3].message == "Upload resptrs of resource 'foo_1_id'..." - assert caplog.records[4].message == 'Successfully uploaded resptr links of "my_onto:hasCustomLink"' - assert caplog.records[5].message == "Upload resptrs of resource 'foo_2_id'..." - assert caplog.records[6].message == 'Successfully uploaded resptr links of "my_onto:hasCustomLink"' + assert caplog.records[3].message == " Upload resptrs of resource 'foo_1_id'..." + assert caplog.records[4].message == ' Successfully uploaded resptr links of "my_onto:hasCustomLink"' + assert caplog.records[5].message == " Upload resptrs of resource 'foo_2_id'..." + assert caplog.records[6].message == ' Successfully uploaded resptr links of "my_onto:hasCustomLink"' caplog.clear() From 9ebf19c21a308f3ed7036a0adce8064c1df00a65 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 09:30:00 +0200 Subject: [PATCH 54/69] test only INFO level --- .../xmlupload/test_resource_creation_integration.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index 1057075b9..2bb1ae371 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -86,7 +86,7 @@ def test_one_resource_without_links() -> None: def test_logging(caplog: pytest.LogCaptureFixture) -> None: - caplog.set_level("DEBUG") + caplog.set_level("INFO") xml_strings = [ '', '', @@ -129,9 +129,7 @@ def test_logging(caplog: pytest.LogCaptureFixture) -> None: xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) assert caplog.records[1].message == "Created resource 5/5: 'foo_5_label' (ID: 'foo_5_id', IRI: 'foo_5_iri')" assert caplog.records[3].message == " Upload resptrs of resource 'foo_1_id'..." - assert caplog.records[4].message == ' Successfully uploaded resptr links of "my_onto:hasCustomLink"' - assert caplog.records[5].message == " Upload resptrs of resource 'foo_2_id'..." - assert caplog.records[6].message == ' Successfully uploaded resptr links of "my_onto:hasCustomLink"' + assert caplog.records[4].message == " Upload resptrs of resource 'foo_2_id'..." caplog.clear() From 7f86b09d2103dc5606e12a99e2f1c121055e9535 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 09:40:50 +0200 Subject: [PATCH 55/69] reset pre-commit config --- .pre-commit-config.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ca991fcdf..54c233a7d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -13,7 +13,6 @@ repos: --ignore=PLR0913, --ignore=PLR2004, --ignore=PLW0603, - --ignore=FIX002, ] - id: ruff-format From f3ae4fa0a7cdb8bcf249ae5816076d19a996eabe Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 09:47:54 +0200 Subject: [PATCH 56/69] docstring --- src/dsp_tools/commands/xmlupload/resource_create_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dsp_tools/commands/xmlupload/resource_create_client.py b/src/dsp_tools/commands/xmlupload/resource_create_client.py index 01c337248..71701289c 100644 --- a/src/dsp_tools/commands/xmlupload/resource_create_client.py +++ b/src/dsp_tools/commands/xmlupload/resource_create_client.py @@ -39,7 +39,7 @@ def create_resource( resource: XMLResource, bitstream_information: BitstreamInfo | None, ) -> str: - """Creates a resource on the DSP server.""" + """Creates a resource on the DSP server, and returns its IRI""" logger.info( f"Attempting to create resource {resource.res_id} (label: {resource.label}, iri: {resource.iri})..." ) From ca77bfa6fc137bf487735af47caf530615d4c7e7 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 09:50:33 +0200 Subject: [PATCH 57/69] align warning class with Nora's feature branch --- src/dsp_tools/models/custom_warnings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dsp_tools/models/custom_warnings.py b/src/dsp_tools/models/custom_warnings.py index 5ab9cab25..43b5c41f5 100644 --- a/src/dsp_tools/models/custom_warnings.py +++ b/src/dsp_tools/models/custom_warnings.py @@ -13,8 +13,8 @@ def showwarning(cls, message: str) -> None: """Functionality that should be executed when a warning of this class is emitted""" -class GeneralDspToolsWarning(Warning): - """Class for user-facing deprecation warnings""" +class DspToolsUserWarning(Warning): + """Class for general user-facing warnings""" @classmethod def showwarning(cls, message: str) -> None: From d6da780cf5d993cd1f71ec97cec5430cb6731a60 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 10:05:19 +0200 Subject: [PATCH 58/69] fix import --- src/dsp_tools/commands/xmlupload/xmlupload.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index 47499736b..8e0f68589 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -33,7 +33,7 @@ from dsp_tools.commands.xmlupload.stash.upload_stashed_xml_texts import upload_stashed_xml_texts from dsp_tools.commands.xmlupload.upload_config import UploadConfig from dsp_tools.commands.xmlupload.write_diagnostic_info import write_id2iri_mapping -from dsp_tools.models.custom_warnings import GeneralDspToolsWarning +from dsp_tools.models.custom_warnings import DspToolsUserWarning from dsp_tools.models.exceptions import BaseError from dsp_tools.models.exceptions import PermanentTimeOutError from dsp_tools.models.exceptions import UserError @@ -358,7 +358,7 @@ def _upload_one_resource( try: iri = _create_resource(resource, media_info, resource_create_client) except (PermanentTimeOutError, KeyboardInterrupt) as err: - warnings.warn(GeneralDspToolsWarning(f"{type(err).__name__}: Tidying up, then exit...")) + warnings.warn(DspToolsUserWarning(f"{type(err).__name__}: Tidying up, then exit...")) msg = ( f"There was a {type(err).__name__} while trying to create resource '{resource.res_id}'.\n" f"It is unclear if the resource '{resource.res_id}' was created successfully or not.\n" @@ -373,7 +373,7 @@ def _upload_one_resource( try: _tidy_up_resource_creation_idempotent(upload_state, iri, resource) except KeyboardInterrupt as err: - warnings.warn(GeneralDspToolsWarning("KeyboardInterrupt: Tidying up, then exit...")) + warnings.warn(DspToolsUserWarning("KeyboardInterrupt: Tidying up, then exit...")) _tidy_up_resource_creation_idempotent(upload_state, iri, resource) raise err from None From 8d827becb1c57a591e6d13817af92a4540eaf9ec Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 10:06:42 +0200 Subject: [PATCH 59/69] add docstring to conftest.py --- test/conftest.py | 9 ++++++++- .../xmlupload/test_resource_creation_integration.py | 3 +-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/test/conftest.py b/test/conftest.py index 65c1050d4..6d184f1a4 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -10,11 +10,18 @@ @pytest.fixture() def caplog(_caplog: pytest.LogCaptureFixture) -> Iterator[pytest.LogCaptureFixture]: # noqa: F811 (redefinition) + """ + The caplog fixture that comes shipped with pytest does not support loguru. + This modified version can be used exactly like the builtin caplog fixture, + which is documented at https://docs.pytest.org/en/latest/how-to/logging.html#caplog-fixture. + Credits: https://www.youtube.com/watch?v=eFdVlyAGeZU + """ + class PropagateHandler(logging.Handler): def emit(self, record: logging.LogRecord) -> None: logging.getLogger(record.name).handle(record) - handler_id = logger.add(PropagateHandler(), format="{message}") + handler_id = logger.add(sink=PropagateHandler(), format="{message}", level="DEBUG") yield _caplog logger.remove(handler_id) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation_integration.py index 2bb1ae371..54780b57b 100644 --- a/test/integration/commands/xmlupload/test_resource_creation_integration.py +++ b/test/integration/commands/xmlupload/test_resource_creation_integration.py @@ -86,7 +86,6 @@ def test_one_resource_without_links() -> None: def test_logging(caplog: pytest.LogCaptureFixture) -> None: - caplog.set_level("INFO") xml_strings = [ '', '', @@ -129,7 +128,7 @@ def test_logging(caplog: pytest.LogCaptureFixture) -> None: xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) assert caplog.records[1].message == "Created resource 5/5: 'foo_5_label' (ID: 'foo_5_id', IRI: 'foo_5_iri')" assert caplog.records[3].message == " Upload resptrs of resource 'foo_1_id'..." - assert caplog.records[4].message == " Upload resptrs of resource 'foo_2_id'..." + assert caplog.records[5].message == " Upload resptrs of resource 'foo_2_id'..." caplog.clear() From 51edfd6bcb89a5c0203a99bed922b5fa35f42010 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 10:12:34 +0200 Subject: [PATCH 60/69] ignore conftest.py file in distribution tests --- .github/workflows/tests-on-push.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests-on-push.yml b/.github/workflows/tests-on-push.yml index 2ed1fd2ff..29eb45c80 100644 --- a/.github/workflows/tests-on-push.yml +++ b/.github/workflows/tests-on-push.yml @@ -108,4 +108,4 @@ jobs: - name: Install python & poetry, build & install wheel, install pytest uses: ./.github/actions/setup-from-wheel - name: distribution tests - run: .dist-test-venv/bin/pytest test/distribution/ + run: .dist-test-venv/bin/pytest --noconftest test/distribution/ From 5116e0a24fe42eaa23d2136e5a837638ad8289bc Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 10:18:45 +0200 Subject: [PATCH 61/69] make darglint happy --- test/conftest.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/conftest.py b/test/conftest.py index 6d184f1a4..36c4951ab 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -15,6 +15,9 @@ def caplog(_caplog: pytest.LogCaptureFixture) -> Iterator[pytest.LogCaptureFixtu This modified version can be used exactly like the builtin caplog fixture, which is documented at https://docs.pytest.org/en/latest/how-to/logging.html#caplog-fixture. Credits: https://www.youtube.com/watch?v=eFdVlyAGeZU + + Yields: + pytest.LogCaptureFixture: The modified caplog fixture. """ class PropagateHandler(logging.Handler): From 68fff6ecd4e4c212cd85f45c476b84339b67ccf6 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 10:22:38 +0200 Subject: [PATCH 62/69] rename --- ...resource_creation_integration.py => test_resource_creation.py} | 0 .../{test_resource_creation_unit.py => test_resource_creation.py} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename test/integration/commands/xmlupload/{test_resource_creation_integration.py => test_resource_creation.py} (100%) rename test/unittests/commands/xmlupload/{test_resource_creation_unit.py => test_resource_creation.py} (100%) diff --git a/test/integration/commands/xmlupload/test_resource_creation_integration.py b/test/integration/commands/xmlupload/test_resource_creation.py similarity index 100% rename from test/integration/commands/xmlupload/test_resource_creation_integration.py rename to test/integration/commands/xmlupload/test_resource_creation.py diff --git a/test/unittests/commands/xmlupload/test_resource_creation_unit.py b/test/unittests/commands/xmlupload/test_resource_creation.py similarity index 100% rename from test/unittests/commands/xmlupload/test_resource_creation_unit.py rename to test/unittests/commands/xmlupload/test_resource_creation.py From 1af1d4948dbfece7fb3ea459f79659b820305635 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 10:42:16 +0200 Subject: [PATCH 63/69] make sure that only XmlUploadInterruptedError is raised --- src/dsp_tools/commands/xmlupload/xmlupload.py | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index 8e0f68589..2492328fb 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -215,9 +215,7 @@ def upload_resources( iri_resolver=upload_state.iri_resolver, project_client=project_client, ) - except BaseException as err: # noqa: BLE001 (blind-except) - # The forseeable errors are already handled by failed_uploads - # Here we catch the unforseeable exceptions, incl. keyboard interrupt. + except XmlUploadInterruptedError as err: _handle_upload_error(err, upload_state) return None @@ -348,12 +346,19 @@ def _upload_one_resource( resource_create_client: ResourceCreateClient, creation_attempts_of_this_round: int, ) -> None: - success, media_info = handle_media_info( - resource, upload_state.config.media_previously_uploaded, sipi_server, imgdir, upload_state.permissions_lookup - ) - if not success: - upload_state.failed_uploads.append(resource.res_id) - return + try: + success, media_info = handle_media_info( + resource, + upload_state.config.media_previously_uploaded, + sipi_server, + imgdir, + upload_state.permissions_lookup, + ) + if not success: + upload_state.failed_uploads.append(resource.res_id) + return + except KeyboardInterrupt: + raise XmlUploadInterruptedError("KeyboardInterrupt during media file upload") from None try: iri = _create_resource(resource, media_info, resource_create_client) @@ -372,12 +377,11 @@ def _upload_one_resource( try: _tidy_up_resource_creation_idempotent(upload_state, iri, resource) - except KeyboardInterrupt as err: + _interrupt_if_indicated(upload_state, creation_attempts_of_this_round) + except KeyboardInterrupt: warnings.warn(DspToolsUserWarning("KeyboardInterrupt: Tidying up, then exit...")) _tidy_up_resource_creation_idempotent(upload_state, iri, resource) - raise err from None - - _interrupt_if_indicated(upload_state, creation_attempts_of_this_round) + raise XmlUploadInterruptedError("KeyboardInterrupt during tidy up") from None def _interrupt_if_indicated(upload_state: UploadState, creation_attempts_of_this_round: int) -> None: From 8689d3403b4fd448c33903a26374ab0344b9e885 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 10:47:25 +0200 Subject: [PATCH 64/69] cosmetics --- src/dsp_tools/commands/xmlupload/xmlupload.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index 2492328fb..c7aad2978 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -388,9 +388,8 @@ def _interrupt_if_indicated(upload_state: UploadState, creation_attempts_of_this # if the interrupt_after value is not set, the upload will not be interrupted interrupt_after = upload_state.config.interrupt_after or 999_999_999 if creation_attempts_of_this_round + 1 >= interrupt_after: - raise XmlUploadInterruptedError( - f"Interrupted: Maximum number of resources was reached ({upload_state.config.interrupt_after})" - ) + msg = f"Interrupted: Maximum number of resources was reached ({upload_state.config.interrupt_after})" + raise XmlUploadInterruptedError(msg) def _tidy_up_resource_creation_idempotent( From c970197c07580d9c1fe6ad3be3b0d55d1b973bb5 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 12:14:49 +0200 Subject: [PATCH 65/69] rearrange test functions --- .../xmlupload/test_resource_creation.py | 94 +++++++++---------- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/test/integration/commands/xmlupload/test_resource_creation.py b/test/integration/commands/xmlupload/test_resource_creation.py index 54780b57b..4d0cddd7d 100644 --- a/test/integration/commands/xmlupload/test_resource_creation.py +++ b/test/integration/commands/xmlupload/test_resource_creation.py @@ -85,53 +85,6 @@ def test_one_resource_without_links() -> None: assert not upload_state.pending_stash -def test_logging(caplog: pytest.LogCaptureFixture) -> None: - xml_strings = [ - '', - '', - '', - '', - '', - ] - xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] - link_val_stash_dict = { - "foo_1_id": [LinkValueStashItem("foo_1_id", "my_onto:foo_1_type", "my_onto:hasCustomLink", "foo_2_id")], - "foo_2_id": [LinkValueStashItem("foo_2_id", "my_onto:foo_2_type", "my_onto:hasCustomLink", "foo_1_id")], - } - stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) - upload_config = UploadConfig(interrupt_after=2) - upload_state = UploadState(xml_resources.copy(), [], IriResolver(), deepcopy(stash), upload_config, {}) - con = Mock(spec_set=ConnectionLive) - post_responses = [ - {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, - {"@id": "foo_2_iri", "rdfs:label": "foo_2_label"}, - {"@id": "foo_3_iri", "rdfs:label": "foo_3_label"}, - {"@id": "foo_4_iri", "rdfs:label": "foo_4_label"}, - {"@id": "foo_5_iri", "rdfs:label": "foo_5_label"}, - {}, # uploading a stash doesn't rely on a certain response - {}, # uploading a stash doesn't rely on a certain response - ] - con.post = Mock(side_effect=post_responses) - project_client = ProjectClientStub(con, "1234", None) - xmlupload._handle_upload_error = Mock() - - xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) - assert caplog.records[1].message == "Created resource 1/5: 'foo_1_label' (ID: 'foo_1_id', IRI: 'foo_1_iri')" - assert caplog.records[3].message == "Created resource 2/5: 'foo_2_label' (ID: 'foo_2_id', IRI: 'foo_2_iri')" - caplog.clear() - - xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) - assert caplog.records[1].message == "Created resource 3/5: 'foo_3_label' (ID: 'foo_3_id', IRI: 'foo_3_iri')" - assert caplog.records[3].message == "Created resource 4/5: 'foo_4_label' (ID: 'foo_4_id', IRI: 'foo_4_iri')" - caplog.clear() - - xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) - assert caplog.records[1].message == "Created resource 5/5: 'foo_5_label' (ID: 'foo_5_id', IRI: 'foo_5_iri')" - assert caplog.records[3].message == " Upload resptrs of resource 'foo_1_id'..." - assert caplog.records[5].message == " Upload resptrs of resource 'foo_2_id'..." - caplog.clear() - - def test_one_resource_with_link_to_existing_resource() -> None: xml_strings = [ """ @@ -374,6 +327,53 @@ def test_6_resources_with_stash_and_interrupt_after_2() -> None: assert upload_state == upload_state_expected +def test_logging(caplog: pytest.LogCaptureFixture) -> None: + xml_strings = [ + '', + '', + '', + '', + '', + ] + xml_resources = [XMLResource(etree.fromstring(xml_str), "my_onto") for xml_str in xml_strings] + link_val_stash_dict = { + "foo_1_id": [LinkValueStashItem("foo_1_id", "my_onto:foo_1_type", "my_onto:hasCustomLink", "foo_2_id")], + "foo_2_id": [LinkValueStashItem("foo_2_id", "my_onto:foo_2_type", "my_onto:hasCustomLink", "foo_1_id")], + } + stash = Stash(link_value_stash=LinkValueStash(link_val_stash_dict), standoff_stash=None) + upload_config = UploadConfig(interrupt_after=2) + upload_state = UploadState(xml_resources.copy(), [], IriResolver(), deepcopy(stash), upload_config, {}) + con = Mock(spec_set=ConnectionLive) + post_responses = [ + {"@id": "foo_1_iri", "rdfs:label": "foo_1_label"}, + {"@id": "foo_2_iri", "rdfs:label": "foo_2_label"}, + {"@id": "foo_3_iri", "rdfs:label": "foo_3_label"}, + {"@id": "foo_4_iri", "rdfs:label": "foo_4_label"}, + {"@id": "foo_5_iri", "rdfs:label": "foo_5_label"}, + {}, # uploading a stash doesn't rely on a certain response + {}, # uploading a stash doesn't rely on a certain response + ] + con.post = Mock(side_effect=post_responses) + project_client = ProjectClientStub(con, "1234", None) + xmlupload._handle_upload_error = Mock() + + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + assert caplog.records[1].message == "Created resource 1/5: 'foo_1_label' (ID: 'foo_1_id', IRI: 'foo_1_iri')" + assert caplog.records[3].message == "Created resource 2/5: 'foo_2_label' (ID: 'foo_2_id', IRI: 'foo_2_iri')" + caplog.clear() + + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + assert caplog.records[1].message == "Created resource 3/5: 'foo_3_label' (ID: 'foo_3_id', IRI: 'foo_3_iri')" + assert caplog.records[3].message == "Created resource 4/5: 'foo_4_label' (ID: 'foo_4_id', IRI: 'foo_4_iri')" + caplog.clear() + + xmlupload.upload_resources(upload_state, ".", Sipi(con), project_client, ListClientMock()) + assert caplog.records[1].message == "Created resource 5/5: 'foo_5_label' (ID: 'foo_5_id', IRI: 'foo_5_iri')" + assert caplog.records[3].message == " Upload resptrs of resource 'foo_1_id'..." + assert caplog.records[5].message == " Upload resptrs of resource 'foo_2_id'..." + caplog.clear() + + def test_post_requests() -> None: xml_strings = [ '', From b3e01ce11ed225030fc1c97bc76d8e71e851d22e Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 12:33:25 +0200 Subject: [PATCH 66/69] upload_resources(): don't return the nonapplied stash --- .../resume_xmlupload/resume_xmlupload.py | 6 ++- .../stash/upload_stashed_resptr_props.py | 34 ++++++-------- .../stash/upload_stashed_xml_texts.py | 40 +++++++--------- src/dsp_tools/commands/xmlupload/xmlupload.py | 46 +++++-------------- .../stash/test_upload_stash_with_mock.py | 29 +++++------- 5 files changed, 57 insertions(+), 98 deletions(-) diff --git a/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py b/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py index 756464338..f17e8dd98 100644 --- a/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py +++ b/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py @@ -71,7 +71,7 @@ def resume_xmlupload(server: str, user: str, password: str, sipi: str, skip_firs project_client: ProjectClient = ProjectClientLive(con, upload_state.config.shortcode) list_client: ListClient = ListClientLive(con, project_client.get_project_iri()) - nonapplied_stash = upload_resources( + upload_resources( upload_state=upload_state, imgdir=".", sipi_server=sipi_server, @@ -79,7 +79,9 @@ def resume_xmlupload(server: str, user: str, password: str, sipi: str, skip_firs list_client=list_client, ) - return cleanup_upload(upload_state.iri_resolver, upload_state.config, upload_state.failed_uploads, nonapplied_stash) + return cleanup_upload( + upload_state.iri_resolver, upload_state.config, upload_state.failed_uploads, upload_state.pending_stash + ) def _read_upload_state_from_disk(server: str) -> UploadState: diff --git a/src/dsp_tools/commands/xmlupload/stash/upload_stashed_resptr_props.py b/src/dsp_tools/commands/xmlupload/stash/upload_stashed_resptr_props.py index 43b3b9068..ad3c2ec7f 100644 --- a/src/dsp_tools/commands/xmlupload/stash/upload_stashed_resptr_props.py +++ b/src/dsp_tools/commands/xmlupload/stash/upload_stashed_resptr_props.py @@ -2,40 +2,39 @@ from datetime import datetime from typing import Any +from typing import cast from loguru import logger -from dsp_tools.commands.xmlupload.iri_resolver import IriResolver +from dsp_tools.commands.xmlupload.models.upload_state import UploadState from dsp_tools.commands.xmlupload.stash.stash_models import LinkValueStash from dsp_tools.commands.xmlupload.stash.stash_models import LinkValueStashItem +from dsp_tools.commands.xmlupload.stash.stash_models import Stash from dsp_tools.models.exceptions import BaseError from dsp_tools.utils.connection import Connection def upload_stashed_resptr_props( - iri_resolver: IriResolver, + upload_state: UploadState, con: Connection, - stashed_resptr_props: LinkValueStash, context: dict[str, str], -) -> LinkValueStash | None: +) -> None: """ After all resources are uploaded, the stashed resptr props must be applied to their resources in DSP. + The upload state is updated accordingly, as a side effect. Args: - iri_resolver: resolver with a mapping of ids from the XML file to IRIs in DSP + upload_state: the current state of the upload con: connection to DSP - stashed_resptr_props: all resptr props that have been stashed context: the JSON-LD context of the resource - - Returns: - nonapplied_resptr_props: the resptr props that could not be uploaded """ print(f"{datetime.now()}: Upload the stashed resptrs...") logger.info("Upload the stashed resptrs...") - not_uploaded: list[LinkValueStashItem] = [] - for res_id, stash_items in stashed_resptr_props.res_2_stash_items.copy().items(): - res_iri = iri_resolver.get(res_id) + upload_state.pending_stash = cast(Stash, upload_state.pending_stash) + link_value_stash = cast(LinkValueStash, upload_state.pending_stash.link_value_stash) + for res_id, stash_items in link_value_stash.res_2_stash_items.copy().items(): + res_iri = upload_state.iri_resolver.get(res_id) if not res_iri: # resource could not be uploaded to DSP, so the stash cannot be uploaded either # no action necessary: this resource will remain in nonapplied_resptr_props, @@ -44,16 +43,13 @@ def upload_stashed_resptr_props( print(f"{datetime.now()}: Upload resptrs of resource '{res_id}'...") logger.info(f" Upload resptrs of resource '{res_id}'...") for stash_item in stash_items: - target_iri = iri_resolver.get(stash_item.target_id) + target_iri = upload_state.iri_resolver.get(stash_item.target_id) if not target_iri: continue if _upload_stash_item(stash_item, res_iri, target_iri, con, context): - stashed_resptr_props.res_2_stash_items[res_id].remove(stash_item) - else: - not_uploaded.append(stash_item) - if not stashed_resptr_props.res_2_stash_items[res_id]: - del stashed_resptr_props.res_2_stash_items[res_id] - return LinkValueStash.make(not_uploaded) + link_value_stash.res_2_stash_items[res_id].remove(stash_item) + if not link_value_stash.res_2_stash_items[res_id]: + del link_value_stash.res_2_stash_items[res_id] def _upload_stash_item( diff --git a/src/dsp_tools/commands/xmlupload/stash/upload_stashed_xml_texts.py b/src/dsp_tools/commands/xmlupload/stash/upload_stashed_xml_texts.py index 9dbb02a29..1cc507052 100644 --- a/src/dsp_tools/commands/xmlupload/stash/upload_stashed_xml_texts.py +++ b/src/dsp_tools/commands/xmlupload/stash/upload_stashed_xml_texts.py @@ -2,14 +2,17 @@ from datetime import datetime from typing import Any +from typing import cast from urllib.parse import quote_plus from loguru import logger from dsp_tools.commands.xmlupload.iri_resolver import IriResolver from dsp_tools.commands.xmlupload.models.formatted_text_value import FormattedTextValue +from dsp_tools.commands.xmlupload.models.upload_state import UploadState from dsp_tools.commands.xmlupload.stash.stash_models import StandoffStash from dsp_tools.commands.xmlupload.stash.stash_models import StandoffStashItem +from dsp_tools.commands.xmlupload.stash.stash_models import Stash from dsp_tools.models.exceptions import BaseError from dsp_tools.utils.connection import Connection @@ -93,28 +96,22 @@ def _create_XMLResource_json_object_to_update( return jsonobj -def upload_stashed_xml_texts( - iri_resolver: IriResolver, - con: Connection, - stashed_xml_texts: StandoffStash, -) -> StandoffStash | None: +def upload_stashed_xml_texts(upload_state: UploadState, con: Connection) -> None: """ After all resources are uploaded, the stashed xml texts must be applied to their resources in DSP. + The upload state is updated accordingly, as a side effect. Args: - iri_resolver: resolver to map ids from the XML file to IRIs in DSP + upload_state: the current state of the upload con: connection to DSP - stashed_xml_texts: all xml texts that have been stashed - - Returns: - the xml texts that could not be uploaded """ print(f"{datetime.now()}: Upload the stashed XML texts...") logger.info("Upload the stashed XML texts...") - not_uploaded: list[StandoffStashItem] = [] - for res_id, stash_items in stashed_xml_texts.res_2_stash_items.copy().items(): - res_iri = iri_resolver.get(res_id) + upload_state.pending_stash = cast(Stash, upload_state.pending_stash) + standoff_stash = cast(StandoffStash, upload_state.pending_stash.standoff_stash) + for res_id, stash_items in standoff_stash.res_2_stash_items.copy().items(): + res_iri = upload_state.iri_resolver.get(res_id) if not res_iri: # resource could not be uploaded to DSP, so the stash cannot be uploaded either # no action necessary: this resource will remain in the list of not uploaded stash items, @@ -131,25 +128,20 @@ def upload_stashed_xml_texts( for stash_item in stash_items: value_iri = _get_value_iri(stash_item.prop_name, resource_in_triplestore, stash_item.uuid) if not value_iri: - not_uploaded.append(stash_item) continue - success = _upload_stash_item( + if _upload_stash_item( stash_item=stash_item, res_iri=res_iri, res_type=stash_item.res_type, res_id=res_id, value_iri=value_iri, - iri_resolver=iri_resolver, + iri_resolver=upload_state.iri_resolver, con=con, context=context, - ) - if success: - stashed_xml_texts.res_2_stash_items[res_id].remove(stash_item) - else: - not_uploaded.append(stash_item) - if not stashed_xml_texts.res_2_stash_items[res_id]: - stashed_xml_texts.res_2_stash_items.pop(res_id) - return StandoffStash.make(not_uploaded) + ): + standoff_stash.res_2_stash_items[res_id].remove(stash_item) + if not standoff_stash.res_2_stash_items[res_id]: + standoff_stash.res_2_stash_items.pop(res_id) def _get_value_iri( diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index c7aad2978..4b8f58ff1 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -109,7 +109,7 @@ def xmlupload( list_client: ListClient = ListClientLive(con, project_client.get_project_iri()) upload_state = UploadState(resources, [], IriResolver(), stash, config, permissions_lookup) - nonapplied_stash = upload_resources( + upload_resources( upload_state=upload_state, imgdir=imgdir, sipi_server=sipi_server, @@ -117,7 +117,7 @@ def xmlupload( list_client=list_client, ) - return cleanup_upload(upload_state.iri_resolver, config, upload_state.failed_uploads, nonapplied_stash) + return cleanup_upload(upload_state.iri_resolver, config, upload_state.failed_uploads, upload_state.pending_stash) def cleanup_upload( @@ -186,7 +186,7 @@ def upload_resources( sipi_server: Sipi, project_client: ProjectClient, list_client: ListClient, -) -> Stash | None: +) -> None: """ Actual upload of all resources to DSP. @@ -196,9 +196,6 @@ def upload_resources( sipi_server: Sipi instance project_client: a client for HTTP communication with the DSP-API list_client: a client for HTTP communication with the DSP-API - - Returns: - the stash items that could not be reapplied. """ try: _upload_resources( @@ -208,16 +205,10 @@ def upload_resources( project_client=project_client, list_client=list_client, ) - if not upload_state.pending_stash: - return None - return _upload_stash( - stash=upload_state.pending_stash, - iri_resolver=upload_state.iri_resolver, - project_client=project_client, - ) + if upload_state.pending_stash: + _upload_stash(upload_state, project_client) except XmlUploadInterruptedError as err: _handle_upload_error(err, upload_state) - return None def _get_data_from_xml( @@ -233,29 +224,14 @@ def _get_data_from_xml( def _upload_stash( - stash: Stash, - iri_resolver: IriResolver, + upload_state: UploadState, project_client: ProjectClient, -) -> Stash | None: - if stash.standoff_stash: - nonapplied_standoff = upload_stashed_xml_texts( - iri_resolver=iri_resolver, - con=project_client.con, - stashed_xml_texts=stash.standoff_stash, - ) - else: - nonapplied_standoff = None +) -> None: + if upload_state.pending_stash and upload_state.pending_stash.standoff_stash: + upload_stashed_xml_texts(upload_state, project_client.con) context = get_json_ld_context_for_project(project_client.get_ontology_name_dict()) - if stash.link_value_stash: - nonapplied_resptr_props = upload_stashed_resptr_props( - iri_resolver=iri_resolver, - con=project_client.con, - stashed_resptr_props=stash.link_value_stash, - context=context, - ) - else: - nonapplied_resptr_props = None - return Stash.make(nonapplied_standoff, nonapplied_resptr_props) + if upload_state.pending_stash and upload_state.pending_stash.link_value_stash: + upload_stashed_resptr_props(upload_state, project_client.con, context) def _get_project_context_from_server(connection: Connection) -> ProjectContext: diff --git a/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py b/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py index a5b7512b0..2f419fe6b 100644 --- a/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py +++ b/test/integration/commands/xmlupload/stash/test_upload_stash_with_mock.py @@ -6,12 +6,14 @@ from dsp_tools.commands.xmlupload.iri_resolver import IriResolver from dsp_tools.commands.xmlupload.models.formatted_text_value import FormattedTextValue +from dsp_tools.commands.xmlupload.models.upload_state import UploadState from dsp_tools.commands.xmlupload.project_client import ProjectInfo from dsp_tools.commands.xmlupload.stash.stash_models import LinkValueStash from dsp_tools.commands.xmlupload.stash.stash_models import LinkValueStashItem from dsp_tools.commands.xmlupload.stash.stash_models import StandoffStash from dsp_tools.commands.xmlupload.stash.stash_models import StandoffStashItem from dsp_tools.commands.xmlupload.stash.stash_models import Stash +from dsp_tools.commands.xmlupload.upload_config import UploadConfig from dsp_tools.commands.xmlupload.xmlupload import _upload_stash from dsp_tools.utils.connection import Connection @@ -93,12 +95,9 @@ def test_upload_link_value_stash(self) -> None: } ) con: Connection = ConnectionMock(post_responses=[{}]) - nonapplied = _upload_stash( - stash=stash, - iri_resolver=iri_resolver, - project_client=ProjectClientStub(con, "1234", None), - ) - assert nonapplied is None + upload_state = UploadState([], [], iri_resolver, stash, UploadConfig(), {}) + _upload_stash(upload_state, ProjectClientStub(con, "1234", None)) + assert not upload_state.pending_stash or upload_state.pending_stash.is_empty() class TestUploadTextValueStashes: @@ -141,12 +140,9 @@ def test_upload_text_value_stash(self) -> None: ], put_responses=[{}], ) - nonapplied = _upload_stash( - stash=stash, - iri_resolver=iri_resolver, - project_client=ProjectClientStub(con, "1234", None), - ) - assert nonapplied is None + upload_state = UploadState([], [], iri_resolver, stash, UploadConfig(), {}) + _upload_stash(upload_state, ProjectClientStub(con, "1234", None)) + assert not upload_state.pending_stash or upload_state.pending_stash.is_empty() def test_not_upload_text_value_stash_if_uuid_not_on_value(self) -> None: """ @@ -186,9 +182,6 @@ def test_not_upload_text_value_stash_if_uuid_not_on_value(self) -> None: ], put_responses=[{}], ) - nonapplied = _upload_stash( - stash=stash, - iri_resolver=iri_resolver, - project_client=ProjectClientStub(con, "1234", None), - ) - assert nonapplied == stash + upload_state = UploadState([], [], iri_resolver, stash, UploadConfig(), {}) + _upload_stash(upload_state, ProjectClientStub(con, "1234", None)) + assert upload_state.pending_stash == stash From a6af7a1f7e931801ea188356d9a17715df6b7f93 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 13:40:19 +0200 Subject: [PATCH 67/69] refactor + fix cleanup_upload() --- .../resume_xmlupload/resume_xmlupload.py | 4 +- src/dsp_tools/commands/xmlupload/xmlupload.py | 40 +++++++++---------- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py b/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py index f17e8dd98..3cf7f7a6c 100644 --- a/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py +++ b/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py @@ -79,9 +79,7 @@ def resume_xmlupload(server: str, user: str, password: str, sipi: str, skip_firs list_client=list_client, ) - return cleanup_upload( - upload_state.iri_resolver, upload_state.config, upload_state.failed_uploads, upload_state.pending_stash - ) + return cleanup_upload(upload_state) def _read_upload_state_from_disk(server: str) -> UploadState: diff --git a/src/dsp_tools/commands/xmlupload/xmlupload.py b/src/dsp_tools/commands/xmlupload/xmlupload.py index 4b8f58ff1..048547636 100644 --- a/src/dsp_tools/commands/xmlupload/xmlupload.py +++ b/src/dsp_tools/commands/xmlupload/xmlupload.py @@ -117,44 +117,41 @@ def xmlupload( list_client=list_client, ) - return cleanup_upload(upload_state.iri_resolver, config, upload_state.failed_uploads, upload_state.pending_stash) + return cleanup_upload(upload_state) -def cleanup_upload( - iri_resolver: IriResolver, - config: UploadConfig, - failed_uploads: list[str], - nonapplied_stash: Stash | None, -) -> bool: +def cleanup_upload(upload_state: UploadState) -> bool: """ Write the id2iri mapping to a file and print a message to the console. Args: - iri_resolver: mapping from internal IDs to IRIs - config: the upload configuration - failed_uploads: resources that caused an error when uploading to DSP - nonapplied_stash: the stash items that could not be reapplied + upload_state: the current state of the upload Returns: - success status (deduced from failed_uploads) + success status (deduced from failed_uploads and non-applied stash) """ - write_id2iri_mapping(iri_resolver.lookup, config.diagnostics) - if not failed_uploads and not nonapplied_stash: + write_id2iri_mapping(upload_state.iri_resolver.lookup, upload_state.config.diagnostics) + has_stash_failed = upload_state.pending_stash and not upload_state.pending_stash.is_empty() + if not upload_state.failed_uploads and not has_stash_failed: success = True print(f"{datetime.now()}: All resources have successfully been uploaded.") logger.info("All resources have successfully been uploaded.") else: success = False - if failed_uploads: - print(f"\n{datetime.now()}: WARNING: Could not upload the following resources: {failed_uploads}\n") + if upload_state.failed_uploads: + res_msg = f"Could not upload the following resources: {upload_state.failed_uploads}" + print(f"\n{datetime.now()}: WARNING: {res_msg}\n") print(f"For more information, see the log file: {logger_savepath}\n") - logger.warning(f"Could not upload the following resources: {failed_uploads}") - if nonapplied_stash: - print(f"\n{datetime.now()}: WARNING: Could not reapply the following stash items: {nonapplied_stash}\n") + logger.warning(res_msg) + if has_stash_failed: + stash_msg = f"Could not reapply the following stash items: {upload_state.pending_stash}" + print(f"\n{datetime.now()}: WARNING: {stash_msg}\n") print(f"For more information, see the log file: {logger_savepath}\n") - logger.warning(f"Could not reapply the following stash items: {nonapplied_stash}") + logger.warning(stash_msg) + msg = _save_upload_state(upload_state) + print(msg) - config.diagnostics.save_location.unlink(missing_ok=True) + upload_state.config.diagnostics.save_location.unlink(missing_ok=True) return success @@ -466,4 +463,5 @@ def _save_upload_state(upload_state: UploadState) -> str: save_location.touch(exist_ok=True) with open(save_location, "wb") as file: pickle.dump(upload_state, file) + logger.info(f"Saved the current upload state to {save_location}") return f"Saved the current upload state to {save_location}.\n" From ce78f61e02ff6771ace0d075697f5a27c352b9ec Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Tue, 30 Apr 2024 14:02:26 +0200 Subject: [PATCH 68/69] refactor resume_xmlupload() --- .../resume_xmlupload/resume_xmlupload.py | 60 ++++++++++--------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py b/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py index 3cf7f7a6c..6729fc460 100644 --- a/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py +++ b/src/dsp_tools/commands/resume_xmlupload/resume_xmlupload.py @@ -1,7 +1,5 @@ import pickle import sys -from copy import deepcopy -from dataclasses import replace from loguru import logger from termcolor import colored @@ -35,33 +33,9 @@ def resume_xmlupload(server: str, user: str, password: str, sipi: str, skip_firs """ upload_state = _read_upload_state_from_disk(server) if skip_first_resource: - if len(upload_state.pending_resources) > 0: - new_pending = deepcopy(upload_state.pending_resources) - new_pending.pop(0) - upload_state = replace(upload_state, pending_resources=new_pending) - else: - msg = ( - "The list of pending resources is empty.\n" - "It is not yet possible to skip the first item of the stashed properties.\n" - "Do you want to continue with the upload of the stashed properties anyway? [y/n]" - ) - resp = None - while resp not in ["y", "n"]: - resp = input(colored(msg, color="red")) - if resp == "n": - sys.exit(1) + _skip_first_resource(upload_state) - previous_successful = len(upload_state.iri_resolver.lookup) - previous_failed = len(upload_state.failed_uploads) - previous_total = previous_successful + previous_failed - msg = ( - f"Resuming upload for project {upload_state.config.shortcode} on server {server}. " - f"Number of resources uploaded until now: {previous_total}" - ) - if previous_failed: - msg += f" ({previous_failed} of them failed)" - logger.info(msg) - print("\n==========================\n" + msg + "\n==========================\n") + _print_and_log(upload_state, server) con = ConnectionLive(server) con.login(user, password) @@ -87,3 +61,33 @@ def _read_upload_state_from_disk(server: str) -> UploadState: with open(save_location, "rb") as f: saved_state: UploadState = pickle.load(f) # noqa: S301 (deserialization of untrusted data) return saved_state + + +def _skip_first_resource(upload_state: UploadState) -> None: + if len(upload_state.pending_resources) > 0: + upload_state.pending_resources.pop(0) + else: + msg = ( + "The list of pending resources is empty.\n" + "It is not yet possible to skip the first item of the stashed properties.\n" + "Do you want to continue with the upload of the stashed properties anyway? [y/n]" + ) + resp = None + while resp not in ["y", "n"]: + resp = input(colored(msg, color="red")) + if resp == "n": + sys.exit(1) + + +def _print_and_log(upload_state: UploadState, server: str) -> None: + previous_successful = len(upload_state.iri_resolver.lookup) + previous_failed = len(upload_state.failed_uploads) + previous_total = previous_successful + previous_failed + msg = ( + f"Resuming upload for project {upload_state.config.shortcode} on server {server}. " + f"Number of resources uploaded until now: {previous_total}" + ) + if previous_failed: + msg += f" ({previous_failed} of them failed)" + logger.info(msg) + print("\n==========================\n" + msg + "\n==========================\n") From dfcbdeaf5a87588ebf54d6661a562f7878f1f387 Mon Sep 17 00:00:00 2001 From: Johannes Nussbaum Date: Wed, 8 May 2024 10:34:18 +0200 Subject: [PATCH 69/69] add inline comment --- .github/workflows/tests-on-push.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/tests-on-push.yml b/.github/workflows/tests-on-push.yml index 584013cb6..2e38e89a7 100644 --- a/.github/workflows/tests-on-push.yml +++ b/.github/workflows/tests-on-push.yml @@ -109,3 +109,5 @@ jobs: uses: ./.github/actions/setup-from-wheel - name: distribution tests run: .dist-test-venv/bin/pytest --noconftest test/distribution/ + # Reason for the --noconftest flag: + # test/conftest.py configures logging, but loguru should not be installed for the distribution tests