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
test: fix storage benchwrapper to not require an emulator #7143
Conversation
NOTE: when I tried to run the benchmark off of this branch, it failed to build. This doesn't seem related to my change.
|
@@ -32,10 +32,16 @@ | |||
private Storage client; | |||
|
|||
public StorageBenchWrapperImpl(String storageEmulatorHost) { | |||
client = StorageOptions.newBuilder() | |||
if (storageEmulatorHost.isEmpty()){ |
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.
Normally this would be a null check (or both null and empty) versus an empty string
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.
Thanks. I thought isEmpty also did null, but this may be me comparing it to isNullOrEmpty in .NET
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.
Only one nit.
google-cloud-testing/storage-benchwrapper/src/main/java/com/google/cloud/benchwrapper/Main.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.
@chingor13 has a null check change request.
Overall LGTM otherwise.
Extends the java benchwrapper to support live benchmarking.