Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

119 missing files #120

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

119 missing files #120

wants to merge 13 commits into from

Conversation

kevinphippsstfc
Copy link
Contributor

This is (I believe) the core of the work to fix #119

I have so far only made the changes I think are required and not tested them, as I would like to check that the way I am doing this is acceptable before I spend a lot of time doing testing.

If the allowRestoreFailures property is not set the functionality should be unchanged.

If it is set, the changes so far should allow restore requests containing Datasets or Datafiles that do not exist on the archive storage to progress all the way through to passing the isPrepared check or returning ONLINE for getStatus. Only if all the requested items were not found would an Exception be thrown.

Note that there are no changes yet to allow a "failures" file to be added to the downloaded zip.

@kevinphippsstfc
Copy link
Contributor Author

Having added some minor checks and corrections I can confirm that the tests all now pass.

@RKrahl
Copy link
Member

RKrahl commented Mar 31, 2021

Dear @kevinphippsstfc, as discussed in the last meeting, I am very skeptic about this. Since the very beginning of its inception, ids.server has been built on the assumption that the files in the storage are in sync with the Datafiles in ICAT. This assumption is builtin throughout the code. If you break that assumption, ids.server will behave inconsistently.

Since version 1.9.0, as the result of #83, there is an easy way to maintain the consistency between ICAT and storage even in the case that some files are lost in the storage: you only need to clear the location attribute in the affected Datafiles in ICAT and ids.server will simply ignore them. Note that the location attribute has always been reserved for the internal purpose of ids.server.

At the current stage of the code, I am not able to assess the consequences of this change. I would therefore prefer to do #109 first before considering your proposal.

@RKrahl RKrahl added this to the 2.0.0 milestone Mar 31, 2021
@kevinphippsstfc
Copy link
Contributor Author

As I've now completed the work to handle missing files when downloading the zip and added a new test class, I've added this to the pull request. All the unit and integration tests pass but the new functionality still needs some manual testing on a real/preprod ICAT installation. I won't do any work on that until the path to and timescales for integrating this pull request are clearer.

@RKrahl RKrahl modified the milestones: 2.0.0, 3.0.0 Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option for IDS to be tolerant of restore failures
2 participants