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

docs: simplify api docs generation for comparison #4253

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

consideRatio
Copy link
Member

@consideRatio consideRatio commented Dec 7, 2022

Comment on lines 12 to +13
.. autoconfigurable:: Spawner
:members: options_from_form, poll, start, stop, get_args, get_env, get_state, template_namespace, format_string, create_certs, move_certs
:members:
Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines 19 to +20
.. autoconfigurable:: LocalProcessSpawner
:members:
Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines 19 to +20
.. autoclass:: User
:members: escaped_name

.. attribute:: name

The user's name

.. attribute:: server

The user's Server data object if running, None otherwise.
Has ``ip``, ``port`` attributes.

.. attribute:: spawner

The user's :class:`~.Spawner` instance.
:members:
Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines 12 to +13
.. autoconfigurable:: Service
:members: name, admin, url, api_token, managed, kind, command, cwd, environment, user, oauth_client_id, server, prefix, proxy_spec
:members:
Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines 19 to +20
.. autoconfigurable:: ConfigurableHTTPProxy
:members: debug, auth_token, check_running_interval, api_url, command
:members:
Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines 33 to +34
.. autoconfigurable:: DummyAuthenticator
:members:
Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines 26 to +27
.. autoconfigurable:: PAMAuthenticator
:members:
Copy link
Member Author

Choose a reason for hiding this comment

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

@minrk
Copy link
Member

minrk commented Dec 8, 2022

Nice! I think we probably should use :members: most places. Some members that should be documented are likely inherited, which need to be explicitly included or use :inherited-members:.

Part of this is that I haven't been careful about prefixing private APIs with _, which I should have, because extension endpoints like Spawners have a bit more access to objects than I realized. We should go through the User and Spawner APIs and migrate some things to private APIs, deprecating some public-looking names.

Service has no API because no extension APIs have handles on them, so those API docs are purely for jupyterhub contributor reference.

@minrk
Copy link
Member

minrk commented Dec 16, 2022

After thinking a bit about this, I think we mostly don't want :members: on most of these, as the docs are meant to cover configuring these classes, not their Python APIs, which are only used for developing JupyterHub or custom Spawners/Authenticators.

I think User is probably the class where I've done the most "accidental public API" by exposing attributes that shouldn't be public without a _private prefix.

I think maybe even Spawner and Authenticator should occur in two separate places: one config-only for deployers, and one with public API methods for developers. Because implementation details can be very distracting for someone looking for a configuration reference. WDYT?

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.

None yet

2 participants