Skip to content

Do CLI import in __main__, not __init__ #3547

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Do CLI import in __main__, not __init__ #3547

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 21 additions & 12 deletions httpx/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,29 @@
from ._types import *
from ._urls import *

try:
from ._main import main
except ImportError: # pragma: no cover

def main() -> None: # type: ignore
import sys
# import the _main module lazily so that we only incur the extra import time
# for the CLI dependencies (click, rich, etc.) when intending to run the CLI
# and not on every import of httpx
def __getattr__(name: str): # type: ignore[no-untyped-def]
if name == "main":
try:
from ._main import main
except ImportError: # pragma: no cover

print(
"The httpx command line client could not run because the required "
"dependencies were not installed.\nMake sure you've installed "
"everything with: pip install 'httpx[cli]'"
)
sys.exit(1)
def main() -> None: # type: ignore
import sys

print(
"The httpx command line client could not run because the required "
"dependencies were not installed.\nMake sure you've installed "
"everything with: pip install 'httpx[cli]'"
)
sys.exit(1)

return main

raise AttributeError(f"module {__name__!r} has no attribute {name!r}")


__all__ = [
Expand Down Expand Up @@ -59,7 +69,6 @@ def main() -> None: # type: ignore
"InvalidURL",
"Limits",
"LocalProtocolError",
"main",
Copy link
Contributor

Choose a reason for hiding this comment

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

If assume this causes breaking change (deletion from public api) we may need to keep it as pointer to __main__.main which also lazy import it:

def main():
    from .__main__ import main as orignial_main
    original_main()

by this way, there is no need to update test.

Copy link
Author

Choose a reason for hiding this comment

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

good point, will make this change

Copy link
Author

Choose a reason for hiding this comment

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

Turns out this is a little more complicated, unfortunately. The main function in httpx/_main.py is a click.BaseCommand, not just a typing.Callable. Wrapping it in a new def main(...) still changes the public API, since httpx.main is no longer a click.BaseCommand. This gets exposed by the tests, which attempt to access attributes of a click.BaseCommand object that aren't there for a simple function. To address, I believe we'd have to have httpx.__init__.py import click, which leads to the issue we're trying to get around in the first place, or we'd have to do some funky mocking magic to make main look more like a click.BaseCommand. I'm happy to explore that if that's the direction you'd like this to head, but doesn't feel quite right to me at this point.

In summary, I unfortunately can't think of a good way right now to maintain the current public API and achieve the goal of this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wrapping it in a new def main(...) still changes the public API

Also, it could use PEP562 (module level __getattr__) instead of wrapping:

def __getattr__(name):
    if name == "main":
        from .__main__ import main
        return main
    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, updated. Removed the __main__.py module to keep the diff as small as possible. Can add back, or it can go in a separate PR since it's a separate feature.

Copy link
Author

Choose a reason for hiding this comment

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

@T-256 are there further changes you would like to see?

Copy link
Contributor

Choose a reason for hiding this comment

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

it looks good to me, need other maintainers reviewing to going forward to merge.

"MockTransport",
"NetRCAuth",
"NetworkError",
Expand Down