fixes 'refactor tf-job-operator to match style-guide of libsonnet' #1535
Conversation
|
/assign @pdmack |
|
/assign @ankushagarwal |
|
/assign @cheyang |
|
/retest |
|
/cc @jlewi |
|
What's the motivation for refactoring the TFJob operator spec to use k8s.libsonnet mixins as opposed to just writing the manifest directly? Is this intended to help readability? |
|
Thanks @jlewi - so happy you're back. I'll convert to the style you've suggested above. For tf-serving - should i pick that up since it's similar to tensorboard? |
|
Thanks for the warm regards :) If you could pick up tf-serving that would be great; but I'd suggest letting in @lluunn because I think that was also on his radar. Note to get the tests passed you'll have to update them to work with ks 0.12 see |
|
I think this PR requires ks 0.12. So I don't think this will pass until you make the changes I described here Feel free to ping me on slack if you want to discuss. |
|
/retest |
1 similar comment
|
/retest |
|
Looks like some jsonnet test failures; so I assume an issue with this PR. |
…which defaults to cluster and also passes validation
|
Note - there is a subtle issue in the crd definition - see #1606 |
| namespace:: "test-kf-001", | ||
| local paramsv1alpha1 = { | ||
| name:: "tf-job-operator", | ||
| tfJobImage:: "gcr.io/kubeflow-images-public/tf_operator:v20180226-403", |
There was a problem hiding this comment.
Please change to
gcr.io/kubeflow-images-public/tf_operator:v0.3.0 so it won't collide with #1608
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jlewi 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 |
…1535) * /retest * snapshot * snapshot * snapshot * all resources converted * consolidate rules * formatting, comments * remove test params, env * fixed tf-job-operator test * /retest * /retest * /retest * add scope to crd's and include tests * /retest * /retest * /retest * /retest * bump gke version * snapshot * for params.deploymentScope:cluster the crd should not have any scope which defaults to cluster and also passes validation * /retest
…ubeflow#1535) * /retest * snapshot * snapshot * snapshot * all resources converted * consolidate rules * formatting, comments * remove test params, env * fixed tf-job-operator test * /retest * /retest * /retest * add scope to crd's and include tests * /retest * /retest * /retest * /retest * bump gke version * snapshot * for params.deploymentScope:cluster the crd should not have any scope which defaults to cluster and also passes validation * /retest
Fixes #1534
This change is