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

Make module objects available in all_modules #337

Merged
merged 9 commits into from
Jan 23, 2022
Merged

Conversation

mhils
Copy link
Member

@mhils mhils commented Jan 21, 2022

See #336.

@mhils mhils mentioned this pull request Jan 21, 2022
url = f"http://{opts.host}:{opts.port}"
try:
try:
httpd = pdoc.web.DocServer((opts.host, opts.port or 8080), opts.modules)
Copy link

Choose a reason for hiding this comment

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

that should be port >= 1000 or otherwise linux is going to say no-no

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if I'm following here. It's whatever the user specified or 8080 by default. If you explicitly specify something <1024 on Linux then yes, you will get an error unless you have the required extra permissions.


@cache
def render_search_index(self) -> str:
"""Render the search index. For performance reasons this is always cached."""
all_mods = {}
# Some modules may not be importable, which means that they would raise an RuntimeError
Copy link

Choose a reason for hiding this comment

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

if I understand correctly this is exactly what https://docs.python.org/3/library/importlib.html#importlib.abc.MetaPathFinder.find_spec was designed for

Copy link
Member Author

@mhils mhils Jan 21, 2022

Choose a reason for hiding this comment

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

Not really, things are a bit more complicated here. We use find_spec and friends in pdoc.extract to get a list of all modules and submodules, but that does not guarantee yet that all those submodules are actually importable. If your module looks like this, find_spec will happily take it, but you cannot import it later on.

When pdoc is running as web server, we can do two things:

  1. Import all modules on startup, as we would when running with -o. The first downside is that this can be a bit of a performance hog and pdoc may need multiple seconds to start up. The second downside is that pdoc picks up all submodules, including platform-specific ones (think _windows, _linux, ...). So for some projects you will be greeted with quite a few funny error messages, which isn't ideal either.
  2. Only import modules on-demand. The upside is that this is faster, the downside is that this may crash at runtime. Also, some special parts (for example the search index) do require everything to be loaded, which is what we account for here.

I'm still not decided on which approach is better long-term, a strong argument in favor of 1) is that it's conceptually simpler. Then again, a potential multi-second startup time is a bit painful as well. I've thought about adding a progress bar to ease the pain, but then again we don't want additional dependencies and we don't want our own progressbar implementation. 😛

@mhils mhils marked this pull request as ready for review January 23, 2022 10:32
@mhils mhils merged commit 112595e into mitmproxy:main Jan 23, 2022
@mhils mhils deleted the all-modules branch January 23, 2022 10:32
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 this pull request may close these issues.

2 participants