search

PaddleFlow / paddle-operator

Elastic Deep Learning Training based on Kubernetes by Leveraging EDL and Volcano
Apache License 2.0
31 stars 15 forks source link

PS/Worker definitions in api/* should be pointer #51

Closed shinytang6 closed 3 years ago

shinytang6 commented 3 years ago

According to the k8s api conventions:

Optional fields have the following properties:

They have the +optional comment tag in Go.
They are a pointer type in the Go definition (e.g. AwesomeFlag *SomeFlag) or have a built-in nil value (e.g. maps and slices).
The API server should allow POSTing and PUTing a resource with this field unset.

problem we met: currently PS and Worker is non-pointer, when we json.Marshal a PaddleJob without ps def, will appear something like that which make submission err because of "containers":null

"spec":{"ps":{"template":{"metadata":{"creationTimestamp":null},"spec":{"containers":null}}}

So IMO, api def should be improved according to the conventions.

ref:

  1. https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#optional-vs-required
kuizhiqing commented 3 years ago

good point, we will fix it soon.

kuizhiqing commented 3 years ago

fixed by https://github.com/PaddleFlow/paddle-operator/pull/52