-
Notifications
You must be signed in to change notification settings - Fork 389
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
Fix AWS Glacier/Deep Archive loading of restored objects #759
Conversation
… header combinations while debugging do not seem to align docs, more investigation is needed.
…mented support for File->Load From URL if the user knows the S3 URL, they can paste it there directly. Also aws-java-sdk-v2 version bump from Maven
…ion bump since it affects error messaging and "germane-ity" of this PR. Killing Triple/Tuple in next commit.
@jrobinso Sorry for the last commit, no more are coming in unless you have something to add here. This should be ready to merge and release, imho ;) |
AmazonUtils.s3ObjectAccessResult res = isObjectAccessible(bucket, key); | ||
if (!res.getObjAvailable()) { MessageUtils.showErrorMessage(res.getErrorReason(), null); return; } | ||
} | ||
} catch (NullPointerException npe) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a comment, do we know that the user hasn't logged in solely from the npe? Could there be other reasons for an npe?
Thanks for merging that fast, Jim! Looking forward for next release so I can tell our users to update. I had exactly the same thought about that try-catch, but when I started to implement a Promise I'll fix/refactor that part if it becomes a problem ;) |
I don't mind, just noting it, sometimes the simple solution is best. Appreciate the PR. |
Oh on release, I have a juicebox.js and igv.js release to get out first, then we can look at releasing this. Maybe end of next week or week after, ping me if it drags on past that. |
Up until now, error messages with AWS Glacier were limited to knowing the storage class.
When an object gets "thawed" (restored) from AWS Glacier, it holds the StorageClass (it does not transition to i.e standard). Instead, the header
x-amz-restore
should be parsed accordingly for the different lifecycle stages where the S3 object might be in.While I was at it, I also added support for directly loading those S3 objects from
Load-> From URL
menu, to be consistent in UX/behaviour with the S3 Tree selector... this is a very germane change and goes along with this same PR!Please @igvteam @jrobinso @davideby, would you please expedite this PR merge and release? Otherwise the Glacier object restoration will remain inoperative and confusing to users :-S
/cc @reisingerf @andrewpatto