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 default port substitution work with more schemes #240

Open
dmajda opened this issue Sep 21, 2018 · 5 comments
Open

Make default port substitution work with more schemes #240

dmajda opened this issue Sep 21, 2018 · 5 comments

Comments

@dmajda
Copy link

dmajda commented Sep 21, 2018

We are using YARL with mqtt: and mqtts: URLs, for which default ports are not substituted. This means we have to do the substitution manually in our code. This is fragile and easy to forget.

One possibility is to just add substitutions for these schemes (and possibly other common schemes) to YARL directly. This would be straightforward, but I can imagine it could also be a maintenance headache.

Another way would be allowing to extend the list of substitutions as part of the public API. That would allow anyone to add substitutions they need. Right now, this can already be done by modifying yarl.DEFAULT_PORTS, but this way is not documented and it also acts globally, meaning it would be bad e.g. for a library which uses YARL internally to do this. A parameter used when creating a URL would probably be a nicer solution.

@asvetlov
Copy link
Member

URL('mqtt://whatever') works.
The port is None but you can replace it with your desired default on, say, creating a connection.

I'm open to extending DEFAULT_PORTS though if there is an official document which states that scheme X corresponds to port N by default.

@webknjaz
Copy link
Member

@dmajda
Copy link
Author

dmajda commented Oct 20, 2018

URL('mqtt://whatever') works.
The port is None but you can replace it with your desired default on, say, creating a connection.

I know, but it’s a mild annoyance. We have multiple services communicating over MQTT, which means we have to do this repeatedly in our codebase. (Or we could build a wrapper that would do that automatically, but it’s easy to forget one has to use it.)

@asvetlov
Copy link
Member

Now we have cache_* functions set that modifies the library's global state.
We can support something similar for default ports.

Please feel free to provide a pull request.

@dave-shawley
Copy link

@asvetlov I'm planning on submitting a PR for this to add a few more defaults (eg, postgresql, amqp, etc). Do you have a preference on function names to manipulate the default URI schemes?

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

No branches or pull requests

4 participants