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

Suggestions for API reference doc #554

Closed
23 tasks done
benhoyt opened this issue Jan 10, 2025 · 3 comments · Fixed by #555
Closed
23 tasks done

Suggestions for API reference doc #554

benhoyt opened this issue Jan 10, 2025 · 3 comments · Fixed by #555
Assignees

Comments

@benhoyt
Copy link
Contributor

benhoyt commented Jan 10, 2025

The new API reference page is really good, thanks @IronCore864 for your efforts here. Here are a bunch of suggested minor improvements after a closer look:

  • On reflection, it would be great to have the curl, Go, and Python examples all do the same thing (and state as much under "Using the API"). Per discussion, let's use the Go client example: stop a single service "mysvc". So let's change the Python example to use client.stop_services(['mysvc']).
  • The curl equivalent would be:
$ curl --unix-socket /path/to/.pebble.socket \
       --data '{"action":"stop","services":["mysvc"]}' \
       http://_/v1/services
{"type":"async","status-code":202,"status":"Accepted","change":"42","result":null}
$ curl --unix-socket /path/to/.pebble.socket \
       http://_/v1/changes/42/wait
{"type":"sync","status-code":200,"status":"OK",...}
  • Remove the unnecessary indentation in the Go example.
  • Let's make the socket path /path/to/.pebble.socket like the Python one in all cases:
  • I'm pretty sure RFC 3339 times should have either a Z or a time zone offset, not both. Two of the examples have both.
  • Typo "without providing a the service name" -> "without providing a service name".
  • Is "Work-in-progress" still necessary? It looks like they're all there now, which is great.
  • The order of the categories and URLs within a category is unclear to me. Can we order the categories alphabetically, and the URLs within a category alphabetically?
  • The websocket-task endpoint should probably be under "changes", and that category renamed to "changes and tasks". (I realise it's only used for exec right now.)
  • I think the Manage services description "Services operations, like start/stop/restart a service/services, auto-start default services, and replan." is a bit unwieldy. How about "Perform a service operation such as start, stop, or replan. " (Autostart is kind of deprecated, and we don't need to list all of them here -- that's in the enum.)
  • I don't think "format: int32" is helpful on /v1/logs n. In fact, I think we just drop the "format: int32" everywhere.
  • The description of /v1/health needs to say how the "healthy: true/false" value is calculated. Maybe just a one-sentence summary and link to https://canonical-pebble.readthedocs-hosted.com/en/latest/reference/health-checks/#health-endpoint?
  • On "Health check level", would be good to add "If omitted, aggregate healthy status of checks with any (or no) level.". Similar for /v1/checks "level".
  • For GET /v1/files, it would be good to provide an example of a full raw multipart response for the "read" operation. Is that possible, probably in the description? You can see some examples in ops test_pebble.py.
  • For POST /v1/files action "write", could we have a raw multipart example too?
  • For POST /v1/exec, would be good to say in the description that you then need to use /v1/tasks/{task-id}/control and /v1/tasks/{task-id}/stdio (and /stderr if split-stderr true), where the task-id comes from, and so on.
  • Under /v1/tasks/... "websocket-id", could list possible enum values: "control", "stdio", "stderr".
  • We could fully document the websocket protocol, but IMO we don't need to do that now -- realistically no one will need it at that level. I suggest we just cheat and say something like "The full websocket protocol is not documented. For details, see the Python client code."
  • notice.user_id should be notice.user-id
  • For most parameters we prefer "To specify multiple X, specify X multiple times." However, for GET /v1/notices "types" and "keys" we say "Multiple types can be specified as a comma-separated list." These can be specified multiple times too, which IMO is preferable, and good to be consistent. Can we change to "To specify multiple types, include this parameter multiple times." (or similar)
  • Can we link to Data Formats > Time in the GET /v1/notices "after" description?
  • For the description under POST /v1/identities, could link to https://canonical-pebble.readthedocs-hosted.com/en/latest/reference/identities/ for the format of the identities data?
  • For GET /v1/system-info "boot-id"'s description, I think we should just say what's in the Go client doc comment: "A unique string that represents this boot of the server."

I realise that's a lot of items, but they should all be pretty minor!

CC: @dwilding

@IronCore864
Copy link
Contributor

IronCore864 commented Jan 10, 2025

I had a look at the list above, I will refactor most of the items mentioned above.

There are two points that might need further discussion:

  • The order of the categories and URLs follows pebble help output. The tags are also from the output. Shall we reorganize the categories order alphabetically and change the tags (only changes -> changes and tasks)?
  • Enum values order (under the websocket API): According to @dwilding 's suggestion, all enum values are ordered alphabetically, not logically. Shall we reorder it to [control, stdio, stderr] anyway?

@benhoyt
Copy link
Contributor Author

benhoyt commented Jan 12, 2025

The order of the categories and URLs follows pebble help output. The tags are also from the output. Shall we reorganize the categories order alphabetically and change the tags (only changes -> changes and tasks)?

Yes, I think so. The help output is quite short so the grouping makes more sense there. I think in the reference here it'll be better to use alphabetical. The API reference doesn't need to match pebble help output.

Re "changes -> changes and tasks", I think the latter is better as it connects it to tasks. We could also link https://canonical-pebble.readthedocs-hosted.com/en/latest/reference/changes-and-tasks/ -- we call them 'changes and tasks' quite a bit.

Enum values order (under the websocket API): According to @dwilding 's suggestion, all enum values are ordered alphabetically, not logically. Shall we reorder it to [control, stdio, stderr] anyway?

No, sorry, alphabetically is good here too -- I wasn't thinking about order. Let's do alpha: control, stderr, stdio.

@dwilding
Copy link
Contributor

I have a suggestion about the ordering of the categories (aka tags). See #555 (comment)

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 a pull request may close this issue.

3 participants