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

Add MPIJob v1alpha2 API reference #1899

Merged
merged 1 commit into from
Apr 30, 2020

Conversation

terrytangyuan
Copy link
Member

@terrytangyuan terrytangyuan commented Apr 15, 2020

Part of kubeflow/mpi-operator#217.

Signed-off-by: terrytangyuan [email protected]

@kubeflow-bot
Copy link

This change is Reviewable

@terrytangyuan
Copy link
Member Author

Trying to following instruction here but seeing the error below. Does anyone know how to fix it? What version of gen-crd-api-reference-docs have we used to generate the existing docs?

I0415 12:57:05.106541   52177 main.go:123] parsing go packages in directory github.com/kubeflow/mpi-operator/pkg/apis/kubeflow/v1alpha2
I0415 12:57:05.618382   52177 main.go:225] using package=github.com/kubeflow/mpi-operator/pkg/apis/kubeflow/v1alpha2
F0415 12:57:05.646690   52177 main.go:444] type invalid type has kind=Unsupported which is unhandled

@terrytangyuan
Copy link
Member Author

terrytangyuan commented Apr 15, 2020

cc'ing folks that modified the docs recently @richardsliu @kbhawkey @rlenferink

@kbhawkey
Copy link
Contributor

Hello @terrytangyuan . A couple of questions:

@terrytangyuan
Copy link
Member Author

terrytangyuan commented Apr 15, 2020

@kbhawkey
Copy link
Contributor

Ping @sarahmaddox

@sarahmaddox
Copy link
Contributor

/assign @richardsliu

@richardsliu Do you have any guidance to give here?

@richardsliu
Copy link
Collaborator

@zhenghuiwang How did you work around this issue?

@zhenghuiwang
Copy link
Contributor

@terrytangyuan FYI this is a workaround I tried last time for generating notebook controller API references.

@terrytangyuan
Copy link
Member Author

terrytangyuan commented Apr 16, 2020

Yea I read that and tried something similar - still no luck. Would it be permitted to add the docs manually? There seems to be more work working around the issues than manually writing up the docs. As we aim to graduate v1 API of MPIJob, the API should stay relatively stable so there won't be that much effort updating it later on.

@sarahmaddox
Copy link
Contributor

FYI, please note that you'll need to move the files in this PR, due to PR #1909, which inserted the /en/ element into the directory path for all content files. In other words, that PR moved all content files from content/docs/... to content/en/docs/....

@terrytangyuan
Copy link
Member Author

terrytangyuan commented Apr 23, 2020

@sarahmaddox Thanks for the note.

The problem right now is that the gen-crd-api-reference-docs tool is either not working or requires specific hack/fix for different controllers/projects. It would be good if someone from your team or from community can put together an alternative solution. Otherwise the API docs are not maintainable. To avoid blocking the graduation of operators, we may just add docs manually.

@sarahmaddox
Copy link
Contributor

@terrytangyuan Thanks for the info. +1 to adding the MPIJob API reference manually for now. Looking at the reports on the various issues, it looks as if others are resorting to the same strategy until the gen-crd-api-reference-docs tool is fixed.

At this point, I think it's worth waiting a while to see if the gen-crd-api-reference-docs bugs get fixed, rather than creating an alternative solution for Kubeflow. But if someone in the community has the bandwidth and knowledge to create an alternative, that would be great too. @terrytangyuan can you create an issue in kubeflow/website, summarizing the problem, giving the current status, asking if anyone would like to develop an alternative tool, and linking to the relevant info:

With that issue, we'll have one convenient place to point people too, and it will also provide the opportunity for someone to create the fix if they want to contribute in that way.

@terrytangyuan
Copy link
Member Author

terrytangyuan commented Apr 23, 2020

@sarahmaddox Thanks! I created an issue in #1924. I'll update this PR later with manually generated API docs for MPIJob if it's not fixed by then.

Signed-off-by: terrytangyuan <[email protected]>
@terrytangyuan
Copy link
Member Author

@sarahmaddox I have added the API reference docs manually. Please take a look when you get a chance.

@terrytangyuan terrytangyuan changed the title [WIP] Add MPIJob API reference Add MPIJob API reference Apr 29, 2020
@terrytangyuan terrytangyuan changed the title Add MPIJob API reference Add MPIJob v1alpha2 API reference Apr 29, 2020
@sarahmaddox
Copy link
Contributor

/unassign @richardsliu
/assign

@sarahmaddox
Copy link
Contributor

Thanks @terrytangyuan!
/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sarahmaddox

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

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.

7 participants