-
Notifications
You must be signed in to change notification settings - Fork 0
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
Implement file management on the cache and source dirs #17
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This includes files in the cache and source dirs. There are also a number of other changes in this commit, it got away from me a bit. Changes include: - cache management for processed images - cache management for the source images - the concept of a source file being in use so that it remains on disk throughout processing - processing is simplified to use a standard process pool in the hopes this reduces the number of zombie processes we keep getting - exception handling is revamped
This commit removes the requirement for an MSS profile image to have an associated specimen/index lot/artefact record in Elasticsearch. This is because we rely on the APS part of the MSS to do this check for us and because the IDs used for the images (specifically GUIDs) are not guessable, so a user can't accidentally stumble upon images they shouldn't be able to get to by incrementing the ID. The side effects of this are reduced time processing requests because we don't have to go ask Elasticsearch about the collections and it gives us the ability to serve images not associated with collections through this interface which is very useful!
This is useful for large images for example where the size exceeds the limits specified in the ImageMagick policy.xml file.
We don't know how they will be used so don't wrap them.
We don't use None returns anymore, we use exceptions.
Now it's not a mystery how big the file is.
It's better if we don't use an AsyncExitStack for this as it means we're leaving connections open after we're really done with them. For example, if we end up needing to go to the dams to get the data, we're leaving 2 connections open to the mss even though we're done with them. Instead, we just open each url in turn and close the connection out when we're done with it. Additionally, this commit adds exception handling so that we can track the URL we were attempting to access up the stack.
Because once we've started streaming the body of the response we can't go back and change the headers we've already sent to show a nice error, we basically just have to hang up the phone and stop sending bits. To this end, there's no point in having various options for handling this, there's only really one choice: raise the exception anyway but before we do (cause it will disappear into the ether), log the error. So now when you stream and original and the backend (mss or dams) fails, we get a log of what happened and the user gets the same experience they had before. Neater and simpler to understand/manage/maintain.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains a few bits and bobs:
content-length
header to/original
endpointCloses #8