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.

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

SDK - Fix pipeline metadata serialization #2137

Merged

Conversation

Ark-kun
Copy link
Contributor

@Ark-kun Ark-kun commented Sep 17, 2019

Two PRs have been merged that turned out to be slightly incompatible. This PR fixes the failing tests.
Root causes:

  • The pipeline parameter default values were not properly serialized when constructing the metadata object.
  • The ParameterMeta class did not validate the default value type, so the lack of serialization has not been caught. The ParameterMeta was replaced by InputSpec which has strict type validation.
  • Previously we did not have samples with complex pipeline parameter default values (e.g. lists) that could trigger the failures. Then two samples were added that had complex default values.

This change is Reviewable

Two PRs have been merged that turned out to be slightly incompatible. This PR fixes the failing tests.
Root causes:
* The pipeline parameter default values were not properly serialized when constructing the metadata object.
* The `ParameterMeta` class did not validate the default value type, so the lack of serialization has not been caught. The `ParameterMeta` was replaced by `InputSpec` which has strict type validation.
* Previously we did not have samples with complex pipeline parameter default values (e.g. lists) that could trigger the failures. Then two samples were added that had complex default values.
* Travis does not re-run tests before merging
* Prow does not re-run Travis tests before merging
@Ark-kun Ark-kun force-pushed the SDK---Fix-pipeline-metadata-serialization branch from 6f21c73 to 6a265f3 Compare September 17, 2019 08:08
@numerology
Copy link

/lgtm

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Sep 17, 2019

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Ark-kun

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Ark-kun Ark-kun merged commit 6afb91b into kubeflow:master Sep 17, 2019
@Ark-kun Ark-kun deleted the SDK---Fix-pipeline-metadata-serialization branch September 17, 2019 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants