Skip to content
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.

Sign up for GitHub

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

feat(sdk): Use google.protobuf.Value in v2 for passing parameters. #6804

Merged
merged 26 commits into from
Oct 28, 2021
Merged

feat(sdk): Use google.protobuf.Value in v2 for passing parameters. #6804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
4966b0b
Use google.protobuf.Value in v2 for passing parameters.
neuromage Oct 26, 2021
8f14fb1
retest samples.
neuromage Oct 26, 2021
728f922
Fix tests.
neuromage Oct 27, 2021
5b13589
Merge branch 'master' into protobuf-value.
neuromage Oct 27, 2021
cf3407d
Update release, more cleanup.
neuromage Oct 27, 2021
7e6d18f
Use github.com/kubeflow/pipelines/api from same repo.
neuromage Oct 27, 2021
bde385f
Run go mod tidy
neuromage Oct 27, 2021
ac5f6e3
chore: go mod tidy
Bobgy Oct 27, 2021
fd8ee77
fix v2 compile error and clean up unused code
Bobgy Oct 27, 2021
216944a
pr comments.
neuromage Oct 27, 2021
87319b8
Merge branch 'protobuf-value' of github.com:neuromage/pipelines into …
neuromage Oct 27, 2021
107a492
Merge branch 'master' into protobuf-value
neuromage Oct 27, 2021
df770b8
Merge branch 'master' into protobuf-value
neuromage Oct 27, 2021
9b5e012
update goldens
neuromage Oct 27, 2021
c8d49a2
Fix metadata recording.
neuromage Oct 27, 2021
de1f943
Update kfp mlmd client.
neuromage Oct 27, 2021
bfc380b
fix test again
neuromage Oct 27, 2021
ad3e7f9
another try.
neuromage Oct 27, 2021
917d979
chore: migrate v2 DAG driver input parameters to protobuf.Value + sma…
Bobgy Oct 28, 2021
b0718ec
fix v2 launcher + clean up
Bobgy Oct 28, 2021
1df554a
fix a compile error
Bobgy Oct 28, 2021
24360f2
fix a few more tests
Bobgy Oct 28, 2021
7b9a681
fix number parsing
Bobgy Oct 28, 2021
547747e
clean up
Bobgy Oct 28, 2021
f2426b0
disable cache_v2 test.
neuromage Oct 28, 2021
35f8448
Merge branch 'protobuf-value' of github.com:neuromage/pipelines into …
neuromage Oct 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion api/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,7 @@ module github.com/kubeflow/pipelines/api

go 1.16

require google.golang.org/protobuf v1.27.1
require (
google.golang.org/genproto v0.0.0-20211026145609-4688e4c4e024
google.golang.org/protobuf v1.27.1
)
116 changes: 115 additions & 1 deletion api/go.sum
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion api/v2alpha1/cache_key.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ syntax = "proto3";
option go_package = "github.com/kubeflow/pipelines/api/v2alpha1/go/cachekey";
package ml_pipelines;

import "google/protobuf/any.proto";
// import "google/protobuf/any.proto";
import "google/protobuf/struct.proto";
import "pipeline_spec.proto";

message CacheKey {
Expand All @@ -26,6 +27,7 @@ message CacheKey {
map<string, RuntimeArtifact> outputArtifactsSpec = 3;
map<string, string> outputParametersSpec=4;
ContainerSpec containerSpec=5;
map<string, google.protobuf.Value> input_parameter_values = 6;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: the other fields use camel case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, but proto style guide is explicitly underscore_separated_names for fields. This is what's used everywhere else as well:
https://developers.google.com/protocol-buffers/docs/style#message_and_field_names

My suggestion is to adopt the right style here for long term health, and consider replacing the camel case ones in a future PR.

}

message ContainerSpec {
Expand Down
Loading