Skip to content

Commit 13b8194

Browse files
authored
chore: partially revert 9afe23e (kubeflow#11713)
In 9afe23e we introduced blackend dropping of unknown fields for unmarshalling, but going forward we want to handle this more on a case by case basis. In the case for driver we should drop them because by this point the api server has declare the pipeline spec is acceptable, so the driver should not fail here. As such we keep driver changes, but revert those utilized by the api server. Signed-off-by: Humair Khan <[email protected]>
1 parent 7d8e921 commit 13b8194

File tree

6 files changed

+12
-10
lines changed

6 files changed

+12
-10
lines changed

backend/src/apiserver/server/list_request_util.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"strconv"
2222
"strings"
2323

24+
"github.com/golang/protobuf/jsonpb"
2425
apiv1beta1 "github.com/kubeflow/pipelines/backend/api/v1beta1/go_client"
2526
apiv2beta1 "github.com/kubeflow/pipelines/backend/api/v2beta1/go_client"
2627
"github.com/kubeflow/pipelines/backend/src/apiserver/common"
@@ -147,13 +148,13 @@ func parseAPIFilter(encoded string, apiVersion string) (interface{}, error) {
147148
switch apiVersion {
148149
case "v2beta1":
149150
f := &apiv2beta1.Filter{}
150-
if err := util.UnmarshalString(decoded, f); err != nil {
151+
if err := jsonpb.UnmarshalString(decoded, f); err != nil {
151152
return nil, util.NewInvalidInputError("failed to parse valid filter from %q: %v", encoded, err)
152153
}
153154
return f, nil
154155
case "v1beta1":
155156
f := &apiv1beta1.Filter{}
156-
if err := util.UnmarshalString(decoded, f); err != nil {
157+
if err := jsonpb.UnmarshalString(decoded, f); err != nil {
157158
return nil, util.NewInvalidInputError("failed to parse valid filter from %q: %v", encoded, err)
158159
}
159160
return f, nil

backend/src/common/util/pipelinerun.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"context"
1919
"flag"
2020
"fmt"
21+
"github.com/golang/protobuf/jsonpb"
2122
"sort"
2223
"strings"
2324
"time"
@@ -663,7 +664,7 @@ func collectTaskRunMetricsOrNil(
663664
// ReportRunMetricsRequest as a workaround to hold user's metrics, which is a superset of what
664665
// user can provide.
665666
reportMetricsRequest := new(api.ReportRunMetricsRequest)
666-
err = UnmarshalString(metricsJSON, reportMetricsRequest)
667+
err = jsonpb.UnmarshalString(metricsJSON, reportMetricsRequest)
667668
if err != nil {
668669
// User writes invalid metrics JSON.
669670
// TODO(#1426): report the error back to api server to notify user

backend/src/common/util/workflow.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"github.com/argoproj/argo-workflows/v3/workflow/packer"
3030
"github.com/argoproj/argo-workflows/v3/workflow/validate"
3131
"github.com/golang/glog"
32+
"github.com/golang/protobuf/jsonpb"
3233
api "github.com/kubeflow/pipelines/backend/api/v1beta1/go_client"
3334
exec "github.com/kubeflow/pipelines/backend/src/common"
3435
swfregister "github.com/kubeflow/pipelines/backend/src/crd/pkg/apis/scheduledworkflow"
@@ -516,7 +517,7 @@ func collectNodeMetricsOrNil(runID string, nodeStatus *workflowapi.NodeStatus, r
516517
// ReportRunMetricsRequest as a workaround to hold user's metrics, which is a superset of what
517518
// user can provide.
518519
reportMetricsRequest := new(api.ReportRunMetricsRequest)
519-
err = UnmarshalString(metricsJSON, reportMetricsRequest)
520+
err = jsonpb.UnmarshalString(metricsJSON, reportMetricsRequest)
520521
if err != nil {
521522
// User writes invalid metrics JSON.
522523
// TODO(#1426): report the error back to api server to notify user

backend/src/v2/compiler/argocompiler/container.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@ package argocompiler
1616

1717
import (
1818
"fmt"
19-
"github.com/kubeflow/pipelines/backend/src/common/util"
2019
"os"
2120
"strconv"
2221
"strings"
2322

2423
wfapi "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
2524
"github.com/golang/glog"
25+
"github.com/golang/protobuf/jsonpb"
2626
"github.com/kubeflow/pipelines/api/v2alpha1/go/pipelinespec"
2727
"github.com/kubeflow/pipelines/backend/src/v2/component"
2828
"github.com/kubeflow/pipelines/kubernetes_platform/go/kubernetesplatform"
@@ -419,7 +419,7 @@ func (c *workflowCompiler) addContainerExecutorTemplate(refName string) string {
419419

420420
if kubernetesConfigParam != nil {
421421
k8sExecCfg := &kubernetesplatform.KubernetesExecutorConfig{}
422-
if err := util.UnmarshalString(string(*kubernetesConfigParam.Value), k8sExecCfg); err == nil {
422+
if err := jsonpb.UnmarshalString(string(*kubernetesConfigParam.Value), k8sExecCfg); err == nil {
423423
extendPodMetadata(&executor.Metadata, k8sExecCfg)
424424
}
425425
}

backend/src/v2/compiler/visitor.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ package compiler
2424
import (
2525
"bytes"
2626
"fmt"
27-
"github.com/kubeflow/pipelines/backend/src/common/util"
2827
"sort"
2928

3029
"github.com/golang/protobuf/jsonpb"
@@ -188,7 +187,7 @@ func GetPipelineSpec(job *pipelinespec.PipelineJob) (*pipelinespec.PipelineSpec,
188187
return nil, fmt.Errorf("failed marshal pipeline spec to json: %w", err)
189188
}
190189
spec := &pipelinespec.PipelineSpec{}
191-
if err := util.UnmarshalString(json, spec); err != nil {
190+
if err := jsonpb.UnmarshalString(json, spec); err != nil {
192191
return nil, fmt.Errorf("failed to parse pipeline spec: %v", err)
193192
}
194193
return spec, nil

backend/src/v2/compiler/visitor_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@ package compiler_test
1515

1616
import (
1717
"fmt"
18-
"github.com/kubeflow/pipelines/backend/src/common/util"
1918
"os"
2019
"testing"
2120

21+
"github.com/golang/protobuf/jsonpb"
2222
"github.com/google/go-cmp/cmp"
2323
"github.com/kubeflow/pipelines/api/v2alpha1/go/pipelinespec"
2424
"github.com/kubeflow/pipelines/backend/src/v2/compiler"
@@ -93,7 +93,7 @@ func load(t *testing.T, path string) *pipelinespec.PipelineJob {
9393
}
9494
json := string(content)
9595
job := &pipelinespec.PipelineJob{}
96-
if err := util.UnmarshalString(json, job); err != nil {
96+
if err := jsonpb.UnmarshalString(json, job); err != nil {
9797
t.Errorf("Failed to parse pipeline job, error: %s, job: %v", err, json)
9898
}
9999
return job

0 commit comments

Comments
 (0)