-
Notifications
You must be signed in to change notification settings - Fork 1
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
[#186091347] Create endpoints to clean up a run #13
[#186091347] Create endpoints to clean up a run #13
Conversation
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.
@Sean-DNAstack this is really great work and I am thoroughly impressed. There are a number of challenges that you encountered here and non trivial blocks of code. I have left a number of suggestions to improve readability and scalability.
src/main/java/com/dnastack/wes/storage/GcpBlobStorageClient.java
Outdated
Show resolved
Hide resolved
src/main/java/com/dnastack/wes/storage/LocalBlobStorageClient.java
Outdated
Show resolved
Hide resolved
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.
@Sean-DNAstack This is really coming along! Great work, I have left a few comments and have requested just a few changes
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.
@Sean-DNAstack you are almost there! I left a few comments and requested a few changes :)
e2e-tests/src/main/java/com/dnastack/wes/service/WesE2ETest.java
Outdated
Show resolved
Hide resolved
e2e-tests/src/main/java/com/dnastack/wes/service/WesE2ETest.java
Outdated
Show resolved
Hide resolved
e2e-tests/src/main/java/com/dnastack/wes/service/WesE2ETest.java
Outdated
Show resolved
Hide resolved
e2e-tests/src/main/java/com/dnastack/wes/service/WesE2ETest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/dnastack/wes/storage/client/local/LocalBlobStorageClientTest.java
Outdated
Show resolved
Hide resolved
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.
@Sean-DNAstack LGTM! Great work!
secondary
from the applicable storage client.