-
Notifications
You must be signed in to change notification settings - Fork 231
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
Add java-storage implementation for createEmptyObjects implementaion. #1094
Add java-storage implementation for createEmptyObjects implementaion. #1094
Conversation
/gcbrun |
6195ef8
to
1467538
Compare
/gcbrun |
private boolean canIgnoreExceptionForEmptyObject( | ||
StorageException exceptionOnCreate, StorageResourceId resourceId, CreateObjectOptions options) | ||
throws IOException { | ||
if (errorExtractor.getErrorType(exceptionOnCreate) == ErrorType.RESOURCE_EXHAUSTED |
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.
nit: consider creating a local variable for errorExtractor.getErrorType(exceptionOnCreate). It will make the code a bit cleaner.
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.
Done
* {@code options}, to be used to determine whether it's safe to ignore an exception that was | ||
* thrown when trying to create the object, {@code exceptionOnCreate}. | ||
*/ | ||
private boolean canIgnoreExceptionForEmptyObject( |
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.
We are doing a lot of code duplication. Is there a way to avoid it?
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.
I believe, we can avoid this duplication by refactoring the GoogleCloudStorageImpl at bit. However, given that we might deprecate GoogleCloudStorageImpl in future. I dont see merit in investing in this re-factoring at the moment. Let me know if you have any other thoughts.
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.
What about tests? Code coverage is way too low.
1467538
to
fe6cbfe
Compare
/gcbrun |
b0133b5
to
ea2e8cd
Compare
/gcbrun |
ea2e8cd
to
07d6cfb
Compare
07d6cfb
to
73d7823
Compare
/gcbrun |
|
No description provided.