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

Jump to bottom

Pipeline API should expose an API to set default version for a given pipeline #4049

Closed
lilida opened this issue Jun 22, 2020 · 7 comments · Fixed by #4406
Closed

Pipeline API should expose an API to set default version for a given pipeline #4049

lilida opened this issue Jun 22, 2020 · 7 comments · Fixed by #4406
Assignees
@Bobgy
Labels
area/backend kind/feature status/triaged Whether the issue has been explicitly triaged

Comments

@lilida
Copy link
Contributor

lilida commented Jun 22, 2020

Today, there is no Pipeline API to update the default version id for a given pipeline. Pipeline default version is only automatically updated when a new version is created. Although UpdatePipelineDefaultVersion has been implemented pipeline_store.go, it was not exposed to pipeline service api.
@Ark-kun Ark-kun added area/backend kind/feature status/triaged Whether the issue has been explicitly triaged labels Jun 23, 2020
@lilida lilida changed the title Pipeline API should expose a particular API to set default version for a given pipeline Pipeline API should expose an API to set default version for a given pipeline Jun 23, 2020
@NikeNano NikeNano mentioned this issue Aug 24, 2020
Merged
2 tasks
@Bobgy
Copy link
Contributor

Bobgy commented Aug 25, 2020

@NikeNano let's discuss in the issue first

My questions:
#4406 (comment)

@NikeNano
Copy link
Member

NikeNano commented Aug 28, 2020 •
edited
Loading

I think it make sene to have the possibility to set the default pipeline. Currently the default pipeline is updated when a new pipeline is uploaded to the latest.

We have had problems when development where done uploading new pipelines and runs where done with the latest versions by mistake instead of the last stable version.

However one possibility would also be to not automatically update when a new pipeline is uploaded.

@Bobgy
Copy link
Contributor

Bobgy commented Sep 2, 2020

Right, looks like the behavior you want is to not automatically update when a new pipeline is uploaded, while being able to set default version programmatically

@Bobgy Bobgy assigned Bobgy and unassigned jingzhang36 Sep 2, 2020
@Bobgy
Copy link
Contributor

Bobgy commented Sep 2, 2020 •
edited
Loading

Can you think about a reasonable way to make the new behavior available to you without breaking backward compatibility?
or maybe a generic tag system can be better?

@NikeNano
Copy link
Member

NikeNano commented Sep 7, 2020

Can you think about a reasonable way to make the new behavior available to you without breaking backward compatibility?
or maybe a generic tag system can be better?

I guess we could add it as an option to update the default version when a new pipeline is uploaded and set it a default to. updates. Thus have the same default behaviour as it is today. But I also think it makes sense to be able to change the default without uploading a new pipeline.

@Bobgy
Copy link
Contributor

Bobgy commented Sep 8, 2020

@NikeNano great idea, that sounds reasonable!

I can help you review any PRs if you'd like to contribute.

@NikeNano
Copy link
Member

NikeNano commented Sep 8, 2020

Great! I will make a PR @Bobgy!

@NikeNano NikeNano mentioned this issue Sep 9, 2020
Merged
2 tasks
k8s-ci-robot pushed a commit that referenced this issue Oct 15, 2020
@NikeNano
feat(API): exposing api for setting the default version of pipeline. F…
5742991 
…ixes #4049 (#4406)

* initial work on exposing the default version of pipeline

* update description

* added missing files

* updated api build, unsure if this is correct ...

* updated after feedback

* clean up

* remove empty line

* started to make the integration test

* added integreation test

* fixed build and feedback

* updated the tests

* Updated the test

* new test

* typo

* updated the pipeline default

* updated the pipeline version

* formatting

* error in comparison
k8s-ci-robot pushed a commit that referenced this issue Oct 19, 2020
@NikeNano
feat(backend): allow configuring if default version should be updated…
2317015 
… when uploading new pipeline version. Fixes #4049 (#4476)

* update to fetch remote

* missed to add the description

* fixed merge conflict

* initial work

* fixed test and bug

* updated python client

* clean up

* clean up

* added config default

* fixed bug in API

* moved config  value

* reverted to load from config

* clean up

* Update _client.py

* removed unecessary function and updated after feedback

* missed to save pipeline.proto

* updated the last parts after feedback

* reverted back to use string and env variable

* updated typo

* fix typo in path

* clean up

* removed option in api

* clean up python part

* typo, cant run test locally

* clean up, problems with local env

* clean up missing differences

* reverted proto files

* further clean up

* clean up

* updated after feedback

* Added tests

* error in my defer statement

* Updated the test
@NikeNano NikeNano mentioned this issue Nov 1, 2020
Closed
Jeffwan pushed a commit to Jeffwan/pipelines that referenced this issue Dec 9, 2020
@NikeNano @Jeffwan
feat(API): exposing api for setting the default version of pipeline. F…
9300bea 
…ixes kubeflow#4049 (kubeflow#4406)

* initial work on exposing the default version of pipeline

* update description

* added missing files

* updated api build, unsure if this is correct ...

* updated after feedback

* clean up

* remove empty line

* started to make the integration test

* added integreation test

* fixed build and feedback

* updated the tests

* Updated the test

* new test

* typo

* updated the pipeline default

* updated the pipeline version

* formatting

* error in comparison
Jeffwan pushed a commit to Jeffwan/pipelines that referenced this issue Dec 9, 2020
@NikeNano @Jeffwan
feat(backend): allow configuring if default version should be updated…
a17dc3d 
… when uploading new pipeline version. Fixes kubeflow#4049 (kubeflow#4476)

* update to fetch remote

* missed to add the description

* fixed merge conflict

* initial work

* fixed test and bug

* updated python client

* clean up

* clean up

* added config default

* fixed bug in API

* moved config  value

* reverted to load from config

* clean up

* Update _client.py

* removed unecessary function and updated after feedback

* missed to save pipeline.proto

* updated the last parts after feedback

* reverted back to use string and env variable

* updated typo

* fix typo in path

* clean up

* removed option in api

* clean up python part

* typo, cant run test locally

* clean up, problems with local env

* clean up missing differences

* reverted proto files

* further clean up

* clean up

* updated after feedback

* Added tests

* error in my defer statement

* Updated the test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Bobgy Bobgy

Labels
area/backend kind/feature status/triaged Whether the issue has been explicitly triaged
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(API): exposing api for setting the default version of pipeline. Fixes #4049 NikeNano/pipelines
5 participants
@Ark-kun @lilida @Bobgy @jingzhang36 @NikeNano