User documentation of kiwix-serve

[veloman-yunkan]

Fixes #498.

[kelson42]

@veloman-yunkan I wonder if the doc can be put on Readthedocs.io? Or how it should be made readable for people?

[veloman-yunkan]

This is the first cut.

I didn't document the KIWIX_SERVE_CUSTOMIZED_RESOURCES environment variable since it was introduced in order to simplify the development process and despite some potential for UI customization shouldn't be advertised before we all agree that it is a good idea.

Also, I didn't include any information from the last column of @rgaudin's table from #498 (comment) . Does it have to be reflected in the documentation?

kiwix-serve man page is not (yet) updated by this PR. I think it must be done in the end or as a separate PR.

[veloman-yunkan]

@veloman-yunkan I wonder if the doc can be put on Readthedocs.io?

@kelson42 Yes, similarly to how it was done for libzim. Do we have a kiwix account with readthedocs? Or @mgautierfr used his own?

[kelson42]

@mgautierfr You are best to answer IMO, I'm not informed about an account for kiwix.

[rgaudin]

Thank you @veloman-yunkan 👍

I am concerned by the fact your are using an outdated Sphinx version (3.5.4, approaching 2 years old). Since it was released, major versions 4 and 5 were and now version 6 is in pre-release.

Starting a new documentation project on an old version seems odd.

Additionally, because that version loosely references its Jinja2 dependency, it is not working. Also, the theme you are using must be installed.

So for that version to work, the requirements.txt file should look like:

Sphinx==3.5.4
sphinx-rtd-theme==1.1.1
Jinja2==3.0.3

Note that it doesn't work on Python3.10 but 3.9 is fine.

Another comment is that the doc option doesn't seem to be set on CI builds right? I think we should build it to ensure it continues to pass. But maybe you'll build and upload to readthedocs on a dedicated workflow?

As per content now, it looks great. And no, I didn't expect the caching stuff to be included ; that was for my own use.

If I may suggest a wording change: I prefer OPDS API over New OPDS API as the other one is already called legacy.

[veloman-yunkan]

I am concerned by the fact your are using an outdated Sphinx version (3.5.4, approaching 2 years old).

@rgaudin That is not a deliberate choice. Without having any prior experience with sphinx I just copied the setup from libzim.

[rgaudin]

I am concerned by the fact your are using an outdated Sphinx version (3.5.4, approaching 2 years old).

@rgaudin That is not a deliberate choice. Without having any prior experience with sphinx I just copied the setup from libzim.

Ah Ok. Well I've tested with latest stable and it works as well so you can use:

Sphinx==5.3.0
sphinx-rtd-theme==1.1.1

Actually, if you remove the reference to the theme in conf.py, you can limit it to Sphinx==5.3.0. It would use the new theme which is certainly different but is more readable I believe.

[mgautierfr]

@mgautierfr You are best to answer IMO, I'm not informed about an account for kiwix.

There is no kiwix account. Administration of projects on readthedocs are made through your github account (you can log with them on readthedocs)

Another comment is that the doc option doesn't seem to be set on CI builds right? I think we should build it to ensure it continues to pass. But maybe you'll build and upload to readthedocs on a dedicated workflow?

readthedocs pull and compile the documentation so we don't have any CI to do on our side.

I've just activated the project and create a temporary "hidden" documentation for this branch : https://kiwix-tools.readthedocs.io/en/documentation/

As this doesn't compile for the reason exposed by @rgaudin, this is now a 404 but it will change soon.

About the dependencies versions:

The pinning of sphinx<4 is pretty old. It seems to works on libzim has readthedocs has its own pinning of jinja2<3.1.0.
However, it is not the case for libkiwix and kiwix-tools. This is probably the "hot fix" in readthedocs/readthedocs.org#9038 (comment)

On libkiwix I have fixed this by not pinning sphinx<4 (kiwix/libkiwix#736)
I suggest to remove the sphinx pinning here also for consistency with libkiwix. (But I'm still open for counter arguments to pin our versions)

[mgautierfr]

Great work

About ZIMID : There is a lot of ZIMID referencing the term ZIM name. I think it would be better to rename them to ZIMNAME.

I wonder if we should document all endpoints publicly. Having a documentation for a endpoint de facto create a contract between the developers and the user that they can use the api. Endpoints like /skin, /catch/external and viewer_settings.js are totally private and user should not use them.
In some extend, /suggest and /content are kind of private as they are mainly use by the viewer. We may change what they return without further notice (as we keep the viewer in sync with them).
At least we should explicitly state that no stability is guaranteed for them.

[rgaudin]

I wonder if we should document all endpoints publicly. Having a documentation for a endpoint de facto create a contract between the developers and the user that they can use the api.

I'm in favor of documenting them (kind of the point of the initial ticket) but clearly mark them private or whichever wording implying it being an exposed endpoint but not a supported API.

[mgautierfr]

.... whichever wording implying it being an exposed endpoint but not a supported API.

Yes. I think you point the right thing here : an endpoint may not be part of a API.
It is open question if we should document them. We don't document private method when documenting cpp/python code[*], we may decide to not document private endpoint.

Maybe we can move the private endpoint in a specific page "private endpoints" and move the pulbic api into a dedicated "http public api" page ?

---

[*] We may put comment in the code for us but they are not listed in the generated (html) documentation.

[veloman-yunkan]

Well I've tested with latest stable and it works as well so you can use:

Sphinx==5.3.0
sphinx-rtd-theme==1.1.1

Actually, if you remove the reference to the theme in conf.py, you can limit it to Sphinx==5.3.0. It would use the new theme which is certainly different but is more readable I believe.

@rgaudin Thanks! I preferred keeping the RTD theme, because of its multilevel table-of-contents in the sidebar.

I've just activated the project and create a temporary "hidden" documentation for this branch : https://kiwix-tools.readthedocs.io/en/documentation/

As this doesn't compile for the reason exposed by @rgaudin, this is now a 404 but it will change soon.

@mgautierfr Thank you too! This PR is now live at https://kiwix-tools.readthedocs.io/en/documentation/

[veloman-yunkan]

The concerns related to the public/private endpoint separation have not been addressed. Let's decide which approach we are going to take.

[kelson42]

A badge linking to the doc should be added in the README.md, like on libzim.

[holta]

Great, Thanks All !

[rgaudin]

LGTM ; Do we want to settle the public/private in this PR or in a following one?

[kelson42]

LGTM ; Do we want to settle the public/private in this PR or in a following one?

Here my feedback on this subject. I´m not really confortable with the idea of having two levels (unofficial/official, private/public, ...) of APIs. I'm not principly opposite to it, but it sounds really complicated for what we need and I see potential problems appearing because of this.

I would suggest that we open a dedicated ticket (before merging that one) for the end-points we are not sure to put in public documented API. We could then discuss if:

• We want to keep them and make public
• Improve them and then make public
• Remove them

These 3 options seems to me good enough (but maybe the future discussion will show that I'm wrong).

[rgaudin]

I don't have a strong opinion on whether we should have unsupported endpoints or not. I can see how it gives us flexibility but I can also see how we could allow ourselves to break API at any version, given we document changes and respect semantic versioning.

I do have one on documentation though ; as not documenting all the endpoints (ie. hiding somes) would not solve my use cases. I would definitely prefer documented-yet-marked-unsupported ones.

Documenting means being transparent and transparency helps integration and maintenance. Kiwix-serve is critical to a number of our products and other organization ones.
By documenting an endpoint (even as unsupported), we acknowledge that it exists and can mention its change or removal in Changelog and that will be beneficial to all.

Hiding it won't prevent users from using it because they might find it anyway because they need something that's not in the public API… but it will break without notice and generate frustration and will discourage them from giving feedback.

[rgaudin]

About ZIMID : There is a lot of ZIMID referencing the term ZIM name. I think it would be better to rename them to ZIMNAME.

Some endpoint accepts the ZIM UUID while others accepts the Name (not always the metadata Name as libkiwix generates it in some cases) and even one accepts the Name without the period suffix.

The documentation should be clear about which one(s) is accepted. This is currently not the case.

For instance, it states

• /raw/ZIMNAME/meta/METADATAID (actually accepts Name)
• /catalog/v2/entry/ZIMNAME (actually accepts UUID)

curl https://library.kiwix.org/raw/ubongo_sw_playlist-mziki-wa-ubongo-kids_2021-06/meta/Title
curl https://library.kiwix.org/catalog/v2/entry/00d73467-0b73-08d8-7577-48e1206b7d18

[mgautierfr]

After a rebase (and fixing the conflicts) are we good to merge?

There is still the question of how we document the private endpoints.
The question may be delayed for later (and so we merge the PR) but we MUST answer the question (and update documentation) before the next release.

[kelson42]

After a rebase (and fixing the conflicts) are we good to merge?

There is still the question of how we document the private endpoints. The question may be delayed for later (and so we merge the PR) but we MUST answer the question (and update documentation) before the next release.

#593 is indeed plan in next release 3.5.0. It should not be considered a blocker for this PR.

But rebasing + CI working is mandatory.

In addition https://kiwix-tools.readthedocs.io/en/latest/?badge=latest is empty, not sure this is normal.

[mgautierfr]

In addition kiwix-tools.readthedocs.io/en/latest/?badge=latest is empty, not sure this is normal.

Yes. PR are not included in "latest". Once merged, it should be ok.

[veloman-yunkan]

But rebasing + CI working is mandatory.

Rebasing (combined with some history clean-up) done. CI is "clean" (Packages workflows won't pass until the libkiwix-dev and libzim-dev packages in http://ppa.launchpad.net/kiwixteam/dev/ubuntu are upgraded to newer releases).

[kelson42]

@mgautierfr Good to merge imo