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

Feature branch for moving to Tekton 0.16 #321

Merged
merged 21 commits into from
Nov 13, 2020
Merged

Conversation

Tomcli
Copy link
Member

@Tomcli Tomcli commented Sep 29, 2020

Which issue is resolved by this Pull Request:
Resolves #304

These features will break backward compatibility, need to wait for Openshift pipeline to support Tekton 0.16 before merging.

related tasks:

Description of your changes:
This is an experimental feature branch for some tekton 0.16 features. We will only merge these features when OKD openshift pipeline supports Tekton 0.16+

Environment tested:

  • Python Version (use python --version): 3.7
  • Tekton Version (use tkn version): 0.16.3
  • Kubernetes Version (use kubectl version): 1.17
  • OS (e.g. from /etc/os-release):

@kubeflow-bot
Copy link

This change is Reviewable

@animeshsingh
Copy link
Collaborator

/hold

@k8s-ci-robot k8s-ci-robot added size/M and removed size/S labels Oct 7, 2020
@k8s-ci-robot k8s-ci-robot added size/L and removed size/M labels Oct 7, 2020
sdk/FEATURES.md Outdated Show resolved Hide resolved
sdk/FEATURES.md Outdated Show resolved Hide resolved
sdk/python/kfp_tekton/compiler/compiler.py Outdated Show resolved Hide resolved
@drewbutlerbb4
Copy link
Member

I have a few comments for the conditions.

Also in _group_to_dag_template of sdk/python/kfp_tekton/compiler/compiler.py
I believe we can remove
'apiVersion': tekton_api_version,
from the template since it is no longer used.

Co-authored-by: Andrew Butler <Andrew.Butler@ibm.com>
@Tomcli Tomcli marked this pull request as ready for review November 3, 2020 23:55
@Tomcli
Copy link
Member Author

Tomcli commented Nov 3, 2020

Openshift pipeline 0.16 is available in open source, need to check which IKS openshift is supporting this version.

@Tomcli
Copy link
Member Author

Tomcli commented Nov 11, 2020

@ckadner do you have any comment on this PR? If not we can merge it to master since this PR has been rebased multiple times.

@ckadner
Copy link
Member

ckadner commented Nov 12, 2020

@ckadner do you have any comment on this PR? If not we can merge it to master since this PR has been rebased multiple times.

I think this looks okay. But I see changes from other merged PRs in your PR. Did you try to rebase your PR instead of merging those changes into you PR

@Tomcli
Copy link
Member Author

Tomcli commented Nov 12, 2020

@ckadner do you have any comment on this PR? If not we can merge it to master since this PR has been rebased multiple times.

I think this looks okay. But I see changes from other merged PRs in your PR. Did you try to rebase your PR instead of merging those changes into you PR

At some point there were too many conflicts to rebase on each commit, so I ended up with merging it and solve all the conflicts in a single batch.

Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ckadner, Tomcli

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

@Tomcli
Copy link
Member Author

Tomcli commented Nov 12, 2020

if there's no more objection i will unhold this PR tomorrow morning.

@Tomcli
Copy link
Member Author

Tomcli commented Nov 13, 2020

/unhold

@k8s-ci-robot k8s-ci-robot merged commit 7cd39a7 into kubeflow:master Nov 13, 2020
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.

update to tekton 0.16 for next release
6 participants