Add initial Sphinx docs #747

webknjaz · 2021-11-17T03:49:12Z

PR preview: https://proxypy--747.org.readthedocs.build/en/747/

Notes

Extended Markdown support in Sphinx is implemented via MyST. Start learning about it here: https://myst-parser.readthedocs.io/en/latest/sphinx/intro.html#extend-markdown-with-a-directive.
The docstrings must use RST, there's no way around it. There's a linter called darglint that is integrated with flake8. It is currently set up to require the Sphinx's built-in docstring markup. Although, its rules are disabled atm because of countless violations. If you want to switch to a different style for documenting function/class signatures, you need to change it @ https://github.com/abhinavsingh/proxy.py/blob/develop/.darglint
Most of the modules have a copyright notice in their module docstrings. I recommend changing that to a comment block because those docstring get into the private API per-module pages and there's a lot of pointless duplication because of this.

docs/spelling_wordlist.txt

codecov · 2021-11-17T03:52:55Z

Codecov Report

Merging #747 (a076a60) into develop (9314d05) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head a076a60 differs from pull request most recent head fa89944. Consider uploading reports for the commit fa89944 to get more accurate results

@@             Coverage Diff             @@
##           develop     #747      +/-   ##
===========================================
- Coverage    87.85%   87.84%   -0.01%     
===========================================
  Files          124      124              
  Lines         5393     5390       -3     
  Branches       430      430              
===========================================
- Hits          4738     4735       -3     
  Misses         569      569              
  Partials        86       86

Flag	Coverage Δ
pytest	`87.71% <ø> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
proxy/common/flag.py	`92.48% <ø> (ø)`
proxy/core/acceptor/acceptor.py	`98.36% <ø> (ø)`
proxy/core/acceptor/pool.py	`94.11% <ø> (ø)`
proxy/core/acceptor/threadless.py	`89.28% <ø> (ø)`
proxy/core/base/tcp_server.py	`89.09% <ø> (ø)`
proxy/core/event/queue.py	`100.00% <ø> (ø)`
proxy/http/parser/parser.py	`94.76% <ø> (ø)`
proxy/http/plugin.py	`100.00% <ø> (ø)`
proxy/http/server/plugin.py	`93.93% <ø> (ø)`
proxy/plugin/reverse_proxy.py	`37.33% <ø> (ø)`
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9314d05...fa89944. Read the comment docs.

webknjaz · 2021-11-17T16:22:33Z

Sure, I just didn't have time to do it.

webknjaz · 2021-11-17T18:24:52Z

@abhinavsingh looks like you didn't fully integrate RTD into this repo — I don't see checks reported in this PR.

webknjaz · 2021-11-17T18:36:08Z

docs/requirements.txt

+# This file is autogenerated by pip-compile with python 3.10
+# To update, run:
+#
+#    pip-compile --allow-unsafe --generate-hashes --output-file=docs/requirements.txt --strip-extras docs/requirements.in


@abhinavsingh note that this file is auto-generated and is checked in for reproducibility purposes.

Thank you, I'll spend sometime to try this out. I haven't used this option before, but I see where it's coming from. I have now about a dozen TODOs on my list, thanks to you :D

abhinavsingh · 2021-11-18T00:09:49Z

Sure, I just didn't have time to do it.

No worries, happy to help.

abhinavsingh · 2021-11-18T00:27:21Z

@abhinavsingh looks like you didn't fully integrate RTD into this repo — I don't see checks reported in this PR.

Missed this comment

/home/docs/checkouts/readthedocs.org/user_builds/proxypy/envs/latest/bin/python -m mkdocs build --clean --site-dir _build/html --config-file mkdocs.yml
ERROR   -  Config value: 'site_dir'. Error: The 'site_dir' should not be within the 'docs_dir' as this leads to the build directory being copied into itself and duplicate nested files in the 'site_dir'.(site_dir: '/home/docs/checkouts/readthedocs.org/user_builds/proxypy/checkouts/latest/_build/html', docs_dir: '/home/docs/checkouts/readthedocs.org/user_builds/proxypy/checkouts/latest') 

Aborted with 1 Configuration Errors!

I see following, but may be because it's trying mkdocs? I changed it to Sphinx Htmldir as you suggested.

abhinavsingh · 2021-11-18T00:29:15Z

Also enabled build pull request which was disabled.

webknjaz · 2021-11-18T00:34:26Z

I see following, but may be because it's trying mkdocs? I changed it to Sphinx Htmldir as you suggested.

Yes, mkdocs is not Sphinx.

Also enabled build pull request which was disabled.

I think you could also add it to the branch protection settings.

webknjaz · 2021-11-18T00:42:37Z

Also enabled build pull request which was disabled.

I think you could also add it to the branch protection settings.

Especially taking into account that build is successful on RTD (https://readthedocs.org/projects/proxypy/builds/15307154/) and the preview is here https://proxypy--747.org.readthedocs.build/en/747/.

abhinavsingh · 2021-11-18T00:49:46Z

This is awesome. I am tempted to take a pass at end-to-end doc string once again :)

I am thinking of 2 edits to make the site more accessible:

Customize these top-level names. I understand these are module name, but probably we are better off by just reproducing the directory structure e.g.

- proxy
  - common
  - core
  - ...

Can we somehow auto-expand the menu and keep it above Glossary. I plan to add some tutorial sections, now that we have a doc site, but I think this still belongs up there. Wdyt? I had to look around for where are the class doc strings :)

Essentially, this is what user sees by default (expanded menu):

- proxy
  - common
  - core
  - ...

abhinavsingh · 2021-11-18T00:54:13Z

Most of the modules have a copyright notice in their module docstrings. I recommend changing that to a comment block because those docstring get into the private API per-module pages and there's a lot of pointless duplication because of this.

+1. I noticed the same, redundant header on every page. Will make a pass at it and push a fix.

webknjaz · 2021-11-18T00:59:31Z

@abhinavsingh you don't need to customize the auto-generated private API pages. It's best to add more human-readable at separate pages (starting with splitting the README into several pages).
But if you really want to, you could try changing the templates as documented here https://www.sphinx-doc.org/en/master/man/sphinx-apidoc.html#cmdoption-sphinx-apidoc-0.

On the second note, I recommend checking out the https://diataxis.fr framework — it is a set of principles for structuring the docs sections and other aspects of the content.

And I recommend joining https://www.writethedocs.org/slack/ — it's a community of mostly tech writers but there are channels for Sphinx and Diataxis where you can ask questions and get insights about improving the docs.

As for expanding or putting the Private API on top — don't. It's there mostly for convenience. Many sites hide it or don't create at all. You need to focus on adding more documents, once this is merged. The idea is that you'll add new document names to one of the ToC sections or create additional one at the top and that will show up above all other menu items.

webknjaz · 2021-11-18T01:02:54Z

README.md

@@ -4,6 +4,8 @@
 ⚡ Fast • 🪶 Lightweight • 0️⃣ Dependency • 🔌 Pluggable • 😈 TLS interception • 🔒 DNS-over-HTTPS • 🔥 Poor Man's VPN • ⏪ Reverse & ⏩ Forward • 👮🏿 "Proxy Server" framework • 🌐 "Web Server" framework • ➵ ➶ ➷ ➠ "PubSub" framework • 👷 "Work" acceptor & executor framework
 </p>

+[//]: # (DO-NOT-REMOVE-docs-badges-START)


@abhinavsingh these markers are referenced from docs/index.md. They are used to include parts of the README on the front page, excluding the badges. It's enough for a demo now but you can make more and include things on other (sub)pages later.

webknjaz · 2021-11-18T01:09:05Z

README.md

@@ -969,7 +973,7 @@ response from the server. Start `proxy.py` as:
    --ca-signing-key-file ca-signing-key.pem
 ```

-[![NOTE](https://img.shields.io/static/v1?label=MacOS&message=note&color=yellow)](https://github.com/abhinavsingh/proxy.py#flags) Also provide explicit CA bundle path needed for validation of peer certificates. See `--ca-file` flag.
+[![NOTE](https://img.shields.io/static/v1?label=MacOS&message=note&color=yellow)](https://github.com/abhinavsingh/proxy.py#user-content-flags) Also provide explicit CA bundle path needed for validation of peer certificates. See `--ca-file` flag.


This one is tricky. The #flags anchor doesn't exist in HTML, it's navigated to via JS so the linkcheck builder cannot verify that the link is valid. All of the real anchors are prefixed, this is why this patch was needed.

Thank you, I did notice this change and didn't understand the context. Makes sense coz sphinx kind of steals away ToC from the readme.

webknjaz · 2021-11-18T01:23:48Z

proxy/core/acceptor/threadless.py

-    a client connection as work payload and hooks into the threadless
-    event loop to handle the client connection.
+    Example, :class:`~proxy.core.base.tcp_server.BaseTcpServerHandler`
+    implements :class:`~proxy.core.acceptor.work.Work` protocol. It


@abhinavsingh the leading tilde means that only the last chunk Work will be visible in HTML.

webknjaz · 2021-11-18T01:26:39Z

docs/conf.py

+# The reST default role (used for this markup: `text`) to use for all
+# documents.
+# Ref: python-attrs/attrs#571
+default_role = 'any'


@abhinavsingh this instructs Sphinx to link any text in single backticks to any role that has an object with that name. Practically, it allows you to identify forgotten syntax (Markdown mixup in RST). In RST, it's best to set roles explicitly.

webknjaz · 2021-11-18T01:33:06Z

docs/conf.py

+
+# -- Options for intersphinx extension ---------------------------------------
+
+intersphinx_mapping = {


@abhinavsingh intersphinx allows you to reference objects defined on other Sphinx sites for as long as you list them here. Sphinx will download their objects.inv files with lists of everything exposed.
The built-in way of exploring those objects is:

$ python -m sphinx.ext.intersphinx https://docs.python.org/3/objects.inv

But it's not very user-friendly, so I have a small project displaying this information for several sites I sometimes interact with. It's here: https://webknjaz.github.io/intersphinx-untangled/.

When linking objects from a foreign site, you can type them in as per usual, with their correct role and name. But since objects with the same name may exist on this and other sites, I prefer explicitly predending the references with <site-alias>:. The dict keys below are those aliases you can use to point at objects at a specific site.

webknjaz · 2021-11-18T01:36:33Z

docs/conf.py

+    'pr': (f'{github_repo_url}/pull/%s', 'PR #'),  # noqa: WPS323
+    'commit': (f'{github_repo_url}/commit/%s', ''),  # noqa: WPS323
+    'gh': (f'{github_url}/%s', 'GitHub: '),  # noqa: WPS323
+    'user': (f'{github_sponsors_url}/%s', '@'),  # noqa: WPS323


@abhinavsingh these are simple roles for making external references. For example, you can use user (in RST) in as follows:

:user:`webknjaz`

And that will link to my sponsors page (for users w/o sponsors enabled, GH will automatically redirect to their profiles).
This is useful to credit contributions to people in the changelog, for example.

webknjaz · 2021-11-18T01:38:45Z

docs/conf.py

+_any_role = 'any'
+_py_class_role = 'py:class'
+nitpick_ignore = [
+    (_any_role, '<proxy.HttpProxyBasePlugin>'),


@abhinavsingh these are weird things mostly in single backticks found across the codebase. They all should be fixed one way or another, this is why they are ignored for now. But the action items are the same as for those linting violations for flake8.

webknjaz · 2021-11-18T01:40:15Z

docs/glossary.md

@@ -0,0 +1,25 @@
+(_proxy_py_glossary)=
+# {{ project }} Glossary


@abhinavsingh I declared this variable substitution so you can use it as {{ project }} in Markdown documents and as |project| in RST.

webknjaz · 2021-11-18T01:42:49Z

docs/glossary.md

+```
+
+```{glossary}
+DNS


@abhinavsingh these are declared mostly so that they could be used via a term directive instead of adding them to spelling exclusions. It's also useful for interlinking the same concept from various places. The lists may contain a few aliases as demonstrated for DoH. RST usage example:

:term:`DNS` :term:`a custom DNS mention anchor <DNS>`

All of this is awesomeness. I did notice the glossary section :)

webknjaz · 2021-11-18T01:44:17Z

docs/index.md

+```{toctree}
+:hidden:
+
+Glossary <glossary>


This syntax allows choosing a custom name for the menu item. In most cases, you'd just type in the document name and its title would be used here.

webknjaz · 2021-11-18T01:46:20Z

docs/index.md

+:start-after: (DO-NOT-REMOVE-docs-badges-END)
+```
+
+```{toctree}


@abhinavsingh you can have as many ToC trees as you want. Each may have a caption if needed. :hidden: is needed to exclude from the document render because this is also included in the sidebar and is unnecessary in the doc bottom.

webknjaz · 2021-11-18T01:48:22Z

docs/conf.py

+author = f'{project} project contributors'
+copyright = author  # pylint: disable=redefined-builtin


I chose this style without a date or the names because of the recommendations from the Linux Foundation.
TL;DR it's easier to maintain and doesn't have direct legal implications anyway.

https://linuxfoundation.org/blog/copyright-notices-in-open-source-software-projects/

webknjaz · 2021-11-18T01:51:03Z

docs/spelling_wordlist.txt

@@ -0,0 +1,7 @@
+IPv


@abhinavsingh this could probably be moved to the glossary and linked as a term once it's out of the README.

webknjaz · 2021-11-18T01:55:11Z

tox.ini

+basepython = python3
+commands_pre =
+  # Paramiko:
+  {envpython} -m pip install -r{toxinidir}/requirements-tunnel.txt


This is not in the deps because the docs requirements use a constraints file with hashes and when pip sees at least one hash, it requires all of the packages to be listed with hashes. The one on this line doesn't have hashes and will fail if listed in the same command. It's a hack but it is what it is. If the env deps infra is properly converted to pip-tools managed constraints files, it would be possible to move this to a better place.

webknjaz · 2021-11-18T01:57:13Z

tox.ini

+usedevelop = false
+
+
+[testenv:doctest-docs]


@abhinavsingh with this, you can start adding doctests as part of the docs snippets per https://www.sphinx-doc.org/en/master/usage/extensions/doctest.html. It's important to know that those snippets aren't out-of-date and still work.

webknjaz · 2021-11-18T02:04:55Z

@abhinavsingh you can now use the short link https://proxypy.rtfd.io on the repository front page, I think.

Also, I have a comment about the pictures. The image in the README is not transparent and is not very well combined with the Dark Mode. It is visible both on the docs site and on GitHub (if you switch to dark mode).

I recommend remaking that picture to:

Use a transparent background
Play well with both white page background and black/dark-gray (this may need experimentation to come up with good colors suitable for both cases simultaneously)
Make that image SVG, not PNG

As a side note, think about creating an SVG favicon too — try to avoid committing binary blobs like pictures into Git repos, it's harmful to them. But since SVG is just text/XML, it's fine + it's great for viewing with the 200% zoom that some people have in their browsers.

webknjaz mentioned this pull request Nov 17, 2021

Upgrade the packaging setup #642

Open

webknjaz commented Nov 17, 2021

View reviewed changes

docs/spelling_wordlist.txt Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

webknjaz closed this Nov 17, 2021

webknjaz reopened this Nov 17, 2021

webknjaz force-pushed the docs/init branch from a076a60 to 6c04fbd Compare November 17, 2021 18:33

webknjaz commented Nov 17, 2021

View reviewed changes

webknjaz force-pushed the docs/init branch 10 times, most recently from d2a1097 to e3580b6 Compare November 17, 2021 23:10

webknjaz force-pushed the docs/init branch from e3580b6 to b40bac7 Compare November 18, 2021 00:16

webknjaz force-pushed the docs/init branch 3 times, most recently from b7962b3 to c74c389 Compare November 18, 2021 00:33

Add initial Sphinx docs

fa89944

webknjaz force-pushed the docs/init branch from c74c389 to fa89944 Compare November 18, 2021 00:36

webknjaz commented Nov 18, 2021

View reviewed changes

abhinavsingh merged commit 8052c90 into abhinavsingh:develop Nov 18, 2021

webknjaz commented Nov 18, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial Sphinx docs #747

Add initial Sphinx docs #747

webknjaz commented Nov 17, 2021 •

edited

Loading

codecov bot commented Nov 17, 2021 •

edited

Loading

This comment has been minimized.

webknjaz commented Nov 17, 2021

webknjaz commented Nov 17, 2021

webknjaz Nov 17, 2021

abhinavsingh Nov 17, 2021

abhinavsingh commented Nov 18, 2021

abhinavsingh commented Nov 18, 2021

abhinavsingh commented Nov 18, 2021

webknjaz commented Nov 18, 2021

webknjaz commented Nov 18, 2021 •

edited

Loading

abhinavsingh commented Nov 18, 2021 •

edited

Loading

abhinavsingh commented Nov 18, 2021

webknjaz commented Nov 18, 2021

webknjaz Nov 18, 2021

webknjaz Nov 18, 2021

abhinavsingh Nov 18, 2021

webknjaz Nov 18, 2021

webknjaz Nov 18, 2021

webknjaz Nov 18, 2021

webknjaz Nov 18, 2021

webknjaz Nov 18, 2021

webknjaz Nov 18, 2021

webknjaz Nov 18, 2021

abhinavsingh Nov 18, 2021

webknjaz Nov 18, 2021

webknjaz Nov 18, 2021

webknjaz Nov 18, 2021

webknjaz Nov 18, 2021

webknjaz Nov 18, 2021

webknjaz Nov 18, 2021

webknjaz commented Nov 18, 2021


		# -- Options for intersphinx extension ---------------------------------------

		intersphinx_mapping = {

		@@ -0,0 +1,25 @@
		(_proxy_py_glossary)=
		# {{ project }} Glossary

		author = f'{project} project contributors'
		copyright = author # pylint: disable=redefined-builtin

		usedevelop = false


		[testenv:doctest-docs]

Add initial Sphinx docs #747

Add initial Sphinx docs #747

Conversation

webknjaz commented Nov 17, 2021 • edited Loading

Notes

codecov bot commented Nov 17, 2021 • edited Loading

Codecov Report

This comment has been minimized.

webknjaz commented Nov 17, 2021

webknjaz commented Nov 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhinavsingh commented Nov 18, 2021

abhinavsingh commented Nov 18, 2021

abhinavsingh commented Nov 18, 2021

webknjaz commented Nov 18, 2021

webknjaz commented Nov 18, 2021 • edited Loading

abhinavsingh commented Nov 18, 2021 • edited Loading

abhinavsingh commented Nov 18, 2021

webknjaz commented Nov 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webknjaz commented Nov 18, 2021

webknjaz commented Nov 17, 2021 •

edited

Loading

codecov bot commented Nov 17, 2021 •

edited

Loading

webknjaz commented Nov 18, 2021 •

edited

Loading

abhinavsingh commented Nov 18, 2021 •

edited

Loading