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

Make TF serving component more readable and extendable #1264

Closed
lluunn opened this issue Jul 25, 2018 · 3 comments
Closed

Make TF serving component more readable and extendable #1264

lluunn opened this issue Jul 25, 2018 · 3 comments

Comments

@lluunn
Copy link
Contributor

lluunn commented Jul 25, 2018

https://github.com/kubeflow/kubeflow/blob/master/kubeflow/tf-serving/tf-serving.libsonnet
It's currently not very readable, and adding other features will make it worse (e.g. request logging).

Some previous discussion in #376 .

Here are some example packages: https://github.com/ksonnet/parts/tree/master/incubator
From this I think the general pattern is for each component, define

  1. default param
  2. base
  3. different extensions

We can utilize ksonnet mixins.

@jlewi
Copy link
Contributor

jlewi commented Jul 25, 2018

I think we should avoid mixins.

The direction we are trending in see (#564) is having ks generate emit a complete object that the user can then modify to customize as needed, with some common fields (e.g. parameter) exposed as parameters.

Furthermore, we should see if we can further simplify things by leveraging YAML support in ksonnet rather then representing prototypes using jsonnet.

@jlewi
Copy link
Contributor

jlewi commented Jul 25, 2018

Given #1229 I think the use case is to support things like injecting a config map and sidecar for request logging.

I think there are two patterns we should explore

  1. Using custom admission controllers
  2. Using overlays
    • I think this was introduced by kustomize
    • But I believe its supported in ksonnet with the "+" operator

@lluunn
Copy link
Contributor Author

lluunn commented Sep 27, 2018

#1589 is the new pattern. We can improve on that

@lluunn lluunn closed this as completed Sep 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants