Skip to content
This repository was archived by the owner on Sep 9, 2023. It is now read-only.

Conversation

telpirion
Copy link
Contributor

This PR includes the following:

  • a ValueConverter class, that provides methods for converting Message types to/from protobuf.Value objects
  • three samples that demonstrate the usage of the enhanced types and ValueConverter class
@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Dec 4, 2020
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Dec 4, 2020
@codecov
Copy link

codecov bot commented Dec 4, 2020

Codecov Report

Merging #108 (980e089) into master (f0145cd) will decrease coverage by 11.99%.
The diff coverage is 0.00%.

Impacted file tree graph

@@ Coverage Diff @@ ## master #108 +/- ## ============================================= - Coverage 76.57% 64.57% -12.00%  + Complexity 606 530 -76  ============================================= Files 48 49 +1 Lines 5617 3263 -2354 Branches 69 14 -55 ============================================= - Hits 4301 2107 -2194  + Misses 1227 1077 -150  + Partials 89 79 -10 
Impacted Files Coverage Δ Complexity Δ
...oud/aiplatform/v1beta1/utility/ValueConverter.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...form/v1beta1/stub/EndpointServiceStubSettings.java 79.86% <0.00%> (-9.92%) 20.00% <0.00%> (-1.00%)
...tform/v1beta1/stub/DatasetServiceStubSettings.java 78.49% <0.00%> (-9.35%) 23.00% <0.00%> (-1.00%)
...1beta1/stub/SpecialistPoolServiceStubSettings.java 79.36% <0.00%> (-9.05%) 17.00% <0.00%> (-1.00%)
...form/v1beta1/stub/PipelineServiceStubSettings.java 78.94% <0.00%> (-8.21%) 15.00% <0.00%> (-1.00%)
...rm/v1beta1/stub/PredictionServiceStubSettings.java 79.48% <0.00%> (-8.10%) 11.00% <0.00%> (ø%)
...latform/v1beta1/stub/ModelServiceStubSettings.java 78.33% <0.00%> (-8.00%) 22.00% <0.00%> (-1.00%)
...latform/v1beta1/stub/GrpcMigrationServiceStub.java 77.50% <0.00%> (-7.44%) 9.00% <0.00%> (-1.00%)
...rm/v1beta1/stub/GrpcSpecialistPoolServiceStub.java 83.82% <0.00%> (-7.22%) 12.00% <0.00%> (-1.00%)
...orm/v1beta1/stub/MigrationServiceStubSettings.java 78.12% <0.00%> (-7.01%) 12.00% <0.00%> (-1.00%)
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f0145cd...980e089. Read the comment docs.

@telpirion telpirion marked this pull request as ready for review December 17, 2020 03:59
@telpirion telpirion requested a review from a team December 17, 2020 03:59
@telpirion telpirion requested review from a team as code owners December 17, 2020 03:59
@telpirion telpirion requested a review from chingor13 December 17, 2020 03:59
@generated-files-bot
Copy link

Warning: This pull request is touching the following templated files:

  • samples/snippets/pom.xml
Copy link
Contributor

@chingor13 chingor13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test for the bad input case as well?

* limitations under the License.
*/

package com.google.cloud.aiplatform.utility;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: most other utility packages use util

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment on lines 74 to 78
if (key.contains("multiLabel")) {
actualBoolValueEntry = entry;
} else if (key.contains("modelType")) {
actualStringValueEntry = entry;
} else if (key.contains("budgetMilliNodeHours")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should these checks for key name use equals?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}

Value actualBoolValue = (Value) actualBoolValueEntry.getValue();
Assert.assertEquals(testObjectInputs.getMultiLabel(), actualBoolValue.getBoolValue());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: usually Assert.assertEquals is statically imported

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

private static AutoMlImageClassificationInputs testObjectInputs;

@Before
public void setUp() throws InvalidProtocolBufferException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything in this setup is only used in the one test - you can move it inside there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

…orm/util/ValueConverterTest.java Co-authored-by: yoshi-code-bot <70984784+yoshi-code-bot@users.noreply.github.com>
@telpirion telpirion requested a review from chingor13 December 18, 2020 00:08
@telpirion telpirion merged commit cf0b763 into master Dec 18, 2020
@telpirion telpirion deleted the enhanced-lib2 branch December 18, 2020 00:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes This human has signed the Contributor License Agreement. samples Issues that are directly related to samples.

4 participants