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

Extend studyjob client API #288

Merged
merged 3 commits into from
Dec 16, 2018

Conversation

andreyvelich
Copy link
Member

@andreyvelich andreyvelich commented Dec 12, 2018

I added namespace parameter to studyJob client API functions.
After this we can use the API not only inside containers.


This change is Reviewable

@andreyvelich
Copy link
Member Author

@YujiOshima

Copy link
Member

@hougangliu hougangliu left a comment

Choose a reason for hiding this comment

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

Suggest change namespace == nil to len(namespace) == 0 to handle namespace := []string{};GetWorkerTemplates(namespace...) case

@andreyvelich
Copy link
Member Author

@hougangliu I agree, changed it.

@YujiOshima
Copy link
Contributor

@andreyvelich Thank you. Could you split getting namespace process to another function like func (s *StudyjobClient)getNameSpace(string ns) string?

@andreyvelich
Copy link
Member Author

@YujiOshima Done

@johnugeorge
Copy link
Member

@andreyvelich What would be the scenario for multiple namespaces in the function call?

@andreyvelich
Copy link
Member Author

@johnugeorge This is not for multiple scenario, you can run these functions without namespace, if you can take namespace from container.
For example:
list := GetStudyJobList()
or
list := GetStudyJobList(namespace)

@YujiOshima
Copy link
Contributor

@andreyvelich Great! Thanks!
/lgtm

@YujiOshima
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: YujiOshima

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

@k8s-ci-robot k8s-ci-robot merged commit 28c5b1c into kubeflow:master Dec 16, 2018
@andreyvelich andreyvelich deleted the extend-sj-client-api branch October 6, 2021 00:23
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