Some issues with the 'Calling into Python Web Apps' docs and API

[thebigmunch]

Quick link to the section for reference. The example doesn't work due to WSGIDispatch not actually being available as httpx.WSGIDispatch. Rather, it's available as httpx.dispatch.asgi.WSGIDispatch. Perhaps ASGIDispatch and WSGIDispatch should be available at the top level.

Also, the difference between remote_addr for WSGI and client for ASGI should be explained or shown as an example, as one is a string and the other a 2-tuple. Or perhaps the API/parameter names should be reconsidered. I know they are meant to mirror the spec names, but maybe client_ip ("client IP" used in docstring for both). And either a separate client_port parameter for both classes or get it automatically in both. Something similar could be said about script_name and root_path. Many people reading these docs won't have knowledge of the specs. The different API/parameter names could be a point of confusion. I don't believe the APIs for these classes need to be coupled with the specs. They just need certain information that can be easily and reasonably obtained without any ties to certain terminology. And even if the APIs remain unchanged, they'll probably need extra documentation/explaining unless its expected for users to have to jump into the code.

And, for good measure, a couple grammar/typo mistakes that could be good candidates for somebody's Hacktoberfest PRs (maybe mine):

• Use a given the client address --> Use a given client address.
• Unnecessary/incorrect comma usage in Using httpx as a client, inside test cases., Mocking out external services, during tests or in dev/staging environments., Mount the WSGI or ASGI application at a subpath, by setting script_name (WSGI) or root_path (ASGI). and Use a given the client address for requests, by setting remote_addr (WSGI) or client (ASGI)..

[sethmlarson]

Thanks for opening this comprehensive post! I'm so glad someone is giving our WSGI and ASGI support a run-around! 🙌

From reading this it looks like there are three distinct issues here:

• ASGIDispatch and WSGIDispatch are documented as top-level but aren't exposed at the top level. This is definitely an issue, I'd recommend the route of making both available top-level.

• The API for ASGIDispatch and WSGIDispatch aren't documented enough that they're usable without knowledge of ASGI or WSGI which for most users of those dispatchers will be the case. Our documentation does need a lot of work, we should jot down bullet points of areas that are confusing and then work to have a resolution to each one before we start working on a PR.

• The grammar / commas don't need their own issue. Open a PR once Hacktoberfest starts and it'll be reviewed and merged. :)

If you don't want to get started on the top two right away I'd recommend we open a separate issue in our tracker for each so that others can potentially take up the work. Once created I'll close this issue.

[thebigmunch]

And, as I look back, I see "Review dispatch interface" in the 1.0 blockers issue...

I'll definitely open separate tracking issues for the first 2. I may do those and the 3rd if nobody gets to them first. I'll get my Hacktoberfest PRs no problem either way, so I won't claim them : )

[tomchristie]

I think #407 mostly resolves this.
We should additionally link to the WSGI and ASGI external docs.
Most folks shouldn't need to dive into those, and app=... should be sufficient.

I don't believe the APIs for these classes need to be coupled with the specs. They just need certain information that can be easily and reasonably obtained without any ties to certain terminology.

We could potentially unify root_path/script_name and remote_addr/client, yeah. We'd need to decide on naming/typing. Happy to take suggestions?

mount_path=str, client_addr=str maybe?

It's not 100% obvious if it's better for us to diverge from the specs there in order to have a consistent interface across both dispatch classes, but I could be convinced.

[thebigmunch]

In API design, a parameter should ideally be the smallest, atomic piece of information required from the user that, preferably, doesn't leak internals/implementations.

As for the difference in the API between WSGIDispatch only having address in its API and automatically getting the port and ASGIDispatch having address and port in its API, I only have one question: Is there a technical reason why this is the case or is it just the spec APIs leaking into httpx APIs? If it's just the spec APIs leaking into httpx, I think the httpx APIs need to change to be the same. I do feel pretty strongly that this needs to change in some way.

As for script_name vs root_path, this they get the atomic part correct. What I'll say about this is that ASGI's naming is so much better. Unifying them would only really affect someone familiar with the spec for which the naming changes. But, it might help users unfamiliar with the specs reason things out using just the parameter name. I'm not so strong in my feelings on this one. It'd be nice if some less knowledgeable users saw this and expressed their opinion, but that is probably unlikely.

[florimondmanca]

It'd be nice if some less knowledgeable users saw this and expressed their opinion, but that is probably unlikely.

Just pitching in to say that I’ve seen people launch Twitter polls for these kinds of questions, and it seems to perform quite well. :)

[tomchristie]

If it's just the spec APIs leaking into httpx

You can argue that it's spec APIs leaking into httpx, or argue that it's a case of not obfuscating away details behind a new and different abstraction. Both valid cases.

I think I'm happy with mount_path=str, and client_addr=str in this particular case tho, yeah. (Alternatives could be considered)

[thebigmunch]

argue that it's a case of not obfuscating away details behind a new and different abstraction

I'd argue that the entirety of API design is purposeful obfuscation. The purpose being to create a better user experience. For example, httpx and requests accept data, json, and files parameters which obfuscates that the HTTP protocols want a body of bytes. But that fact is inconsequential to users.

In the case of the *SGIDispatch APIs, it's inconsequential to users that ASGI expects the address and port in one field as a 2-tuple while WSGI expects them in separate fields. It's only meaningful that a user can provide them if desired.

client_addr=str in this particular case tho, yeah. (Alternatives could be considered)

Is this just the address or address:port. If the latter, I'd suggest separate parameters for address and port.

Just pitching in to say that I’ve seen people launch Twitter polls for these kinds of questions, and it seems to perform quite well. :)

Except for those of us who don't use Twitter or any social media : )

[tomchristie]

At the moment this issue is somewhat defunct, since 0.8 dropped sync (and, correspondingly WSGI).

Once #572 is resolved we'll want a WSGIDispatch again. When that's in we can consider if/how we want to unify the two APIs. In the meantime, I've linked to the ASGI docs in #577 to help out here.