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

[WIP]: Add a command to generate docs for types defined in pkg/api/v*/types.go #11253

Closed

Conversation

caesarxuchao
Copy link
Member

Address #10966

Some known issues:

  1. In swagger spec, fields that has type "map" are set to type "any", so the generated docs doesn't set the type of these fields correctly. There are 14 matches of "map" in v1/types.go
    BTW, swagger-ui also suffers from this problem.
  2. Because in swagger 2.0, the fields are represented by "Properties", which is a map, so the order fields are not preserved. Currently I just sort the fields alphabetically.
  3. Links in descriptions are not rendered to markeddown links

Todo:

  1. We can adapt the WriteSwaggerDocFunc() to write the .md docs if Automatic Generation of Swagger Documentation in Types #10933 gets merged

@erictune @bgrant0607 @nikhiljindal

@k8s-bot
Copy link

k8s-bot commented Jul 14, 2015

GCE e2e build/test passed for commit 6c32972dd8d615cbbbeb87d1d9776bb99ec746ed.

@k8s-bot
Copy link

k8s-bot commented Jul 14, 2015

GCE e2e build/test passed for commit a451e43ba4879cb82e667a3eb2ed49208e593821.

@caesarxuchao
Copy link
Member Author

#9097 shows examples output of some libraries that translates swagger-spec to markdown, which looks much nicer than swagger-ui. But currently it doesn't make markdown for models separately.

@andronat
Copy link
Contributor

I also took notice of swagger2markdown. And I also found that there are some UIs that support .md parsing (e.g. Slate)

The concept is quite interesting. I believe that sooner or later we are going to either have UIs that support swagger api parsing, or swagger community will create nice tools to export to various formats (e.g. markdown)

@k8s-bot
Copy link

k8s-bot commented Jul 15, 2015

GCE e2e build/test passed for commit 83b8d9decb5e7280c65e95ec170aacaee47f01be.

@@ -0,0 +1,34 @@
<!-- BEGIN MUNGE: UNVERSIONED_WARNING -->
Copy link
Member

Choose a reason for hiding this comment

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

your mungedocs is out of date - you'll have to re-run it :)

@thockin
Copy link
Member

thockin commented Jul 15, 2015

This is a step forward, but I dislike the exposure of our type names. Specifically, things like "PodSpec" are not actually part of the API. The only thing that is part of the API is the net structure of each of the top-level objects. This applies to client libs, too (@lavalamp).

How hard would it be to in-line sub-object structures into each referring doc? Something like:

###Pod###

---
* apiVersion: 
  * **_type_**: string
  * **_description_**: version of the schema the object should have; see http://releases.k8s.io/HEAD/docs/api-conventions.md#resources
* kind: 
  * **_type_**: string
  * **_description_**: kind of object, in CamelCase; cannot be updated; see http://releases.k8s.io/HEAD/docs/api-conventions.md#types-kinds
* metadata: 
  * **_type_**: [ObjectMeta](ObjectMeta.md)
  * **_description_**: standard object metadata; see http://releases.k8s.io/HEAD/docs/api-conventions.md#metadata
* spec: 
  * **_type_**: [pod.spec](#PodSpec)
  * **_description_**: specification of the desired behavior of the pod; http://releases.k8s.io/HEAD/docs/api-conventions.md#spec-and-status
* status: 
  * **_type_**: [pod.status](#PodStatus)
  * **_description_**: most recently observed status of the pod; populated by the system, read-only; http://releases.k8s.io/HEAD/docs/api-conventions.md#spec-and-status

## pod.spec
* activeDeadlineSeconds: 
  * **_type_**: integer
  * **_description_**: 
* containers: 
  * **_type_**: [][Container](Container.md)
  * **_description_**: list of containers belonging to the pod; cannot be updated; containers cannot currently be added or removed; there must be at least one container in a Pod; see http://releases.k8s.io/HEAD/docs/containers.md
* dnsPolicy: 
  * **_type_**: string
  * **_description_**: DNS policy for containers within the pod; one of 'ClusterFirst' or 'Default'

... and so on. It would make a lot of duplicated info, but it would insulate us from changes to those structs breaking links into these docs.

@caesarxuchao
Copy link
Member Author

@thockin, are you suggesting that we inline the sub-objects, and remove the separate docs for sub-objects, so that the user only see a bunch of docs for the top-level objects? I think this would be good.

@thockin
Copy link
Member

thockin commented Jul 15, 2015

That's what I am suggesting. Docs for Pod, ReplicationController, Service,
Namespace, etc. But not PodSpec, ServiceSpec, PodStatus, PodCondition, and
so on.

On Wed, Jul 15, 2015 at 10:44 AM, Chao Xu notifications@github.com wrote:

@thockin https://github.com/thockin, are you suggesting that we inline
the sub-objects, and remove the separate docs for sub-objects, so that the
user only see a bunch of docs for the top-level objects? I think this would
be good.


Reply to this email directly or view it on GitHub
#11253 (comment)
.

@thockin
Copy link
Member

thockin commented Jul 15, 2015

This is where Brian disagrees with me... wait for it... wait for it....

On Wed, Jul 15, 2015 at 10:57 AM, Tim Hockin thockin@google.com wrote:

That's what I am suggesting. Docs for Pod, ReplicationController,
Service, Namespace, etc. But not PodSpec, ServiceSpec, PodStatus,
PodCondition, and so on.

On Wed, Jul 15, 2015 at 10:44 AM, Chao Xu notifications@github.com
wrote:

@thockin https://github.com/thockin, are you suggesting that we inline
the sub-objects, and remove the separate docs for sub-objects, so that the
user only see a bunch of docs for the top-level objects? I think this would
be good.


Reply to this email directly or view it on GitHub
#11253 (comment)
.

@k8s-bot
Copy link

k8s-bot commented Jul 15, 2015

GCE e2e build/test failed for commit 260d68b.

@lavalamp
Copy link
Member

I dislike the exposure of our type names. Specifically, things like "PodSpec" are not actually part of the API. The only thing that is part of the API is the net structure of each of the top-level objects. This applies to client libs, too (@lavalamp).

I mostly agree; the names are arbitrary, not version-protected, and unimportant in user-facing docs. However the fact that structure is shared across various objects does seem relevant and useful to know.

For the purpose of making a useful go client lib, however, I think these names are fixed and should not change.

<!-- END STRIP_FOR_RELEASE -->

<!-- END MUNGE: UNVERSIONED_WARNING -->
###Capability###
Copy link
Member

Choose a reason for hiding this comment

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

Presumably there are constant values that would be useful to print here?

@bgrant0607
Copy link
Member

cc @smarterclayton

@erictune
Copy link
Member

Tim's point is well taken. If you can make the change he suggests today, lets do that. But lets definitely get something in today so that other people can then begin linking to the generated docs.

@thockin
Copy link
Member

thockin commented Jul 15, 2015

I'll disagree with Daniel for now and we can take it off-line. I agree
with Eric - today is better than perfect.

On Wed, Jul 15, 2015 at 11:19 AM, Eric Tune notifications@github.com
wrote:

Tim's point is well taken. If you can make the change he suggests today,
lets do that. But lets definitely get something in today so that other
people can then begin linking to the generated docs.


Reply to this email directly or view it on GitHub
#11253 (comment)
.

@lavalamp
Copy link
Member

I agree that now is better than perfect.

@erictune
Copy link
Member

@caesarxuchao you decide if you want to try to implement Tim's suggestion or not. Let us know when it is "done".

@caesarxuchao
Copy link
Member Author

I'll implement Tim's suggestion. I'll update the status.

@bgrant0607
Copy link
Member

Swagger maps represented as any is known issue: #4700.

@bgrant0607
Copy link
Member

The all-in-one doc https://docs.openshift.org/latest/rest_api/kubernetes_v1.html#v1-podspec seems pretty nice, though I'd put the operations and models into 2 separate docs. That would make it easier to link to the models from kubectl-oriented docs.

@bgrant0607
Copy link
Member

I agree that we should de-emphasize subobject names. They are visible in the Swagger, also. Putting all the models into one doc would help, as would formatting top-level objects to be more prominent and subobjects to be less so.

@caesarxuchao
Copy link
Member Author

I'm trying the approach in #9097, because it's output looks more decent. But I doubt I can make it today.

@k8s-bot
Copy link

k8s-bot commented Jul 16, 2015

GCE e2e build/test passed for commit fcc94b1.

@mikedanese
Copy link
Member

please reassign when this is no longer WIP.

@caesarxuchao
Copy link
Member Author

Close for now. Superseded by #11357. We may want to revisit this approach because #11357 introduced java dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants