pyzmq icon indicating copy to clipboard operation
pyzmq copied to clipboard

Add provisional implementation for #1081

Open Kentzo opened this issue 8 years ago • 10 comments

Kentzo avatar Sep 08 '17 01:09 Kentzo

Sorry for the delay in reviewing this. I've no specific objection, but it doesn't really seem like pyzmq is the right place for it. What's the benefit of this over, say, using the builtin urlparse for parsing URLs, which gives you scheme, port, etc.?

minrk avatar Nov 03 '17 11:11 minrk

The main benefit is to be able to integrate with various libraries that deal with user input such as CLI toolkits, config parsers, ORM etc. urlparse is too abstract to be useful as it can only represent 2 components of a zeromq endpoint: scheme and netloc. In my development experience I often need to access just a port or just an interface of a TCP endpoint. And there are other, more complex transports.

Although it can be developed as an independent package, having it bundled provide the following benefits:

  • Native support by pyzmq methods, so one can pass an object into bind / connect etc (implementation only needs to wrap arguments with str or repr)
  • Easier to keep it in sync with zeromq API
  • More attention would allow to have one "definitive" implementation

Kentzo avatar Nov 07 '17 00:11 Kentzo

is too abstract to be useful as it can only represent 2 components of a zeromq endpoint: scheme and netloc

that's not generally true, urlparse extracts host and port as well:

parsed  = urlparse('tcp://127.0.0.1:5555')
parsed.port # 5555
parsed.hostname # '127.0.0.1'

but that doesn't cover more complex non-url transports, such as the multi-endpoint URLs separated by ;.

This is fine by me. What more do you want to do for this PR? Can you add docs and tests?

minrk avatar Nov 07 '17 09:11 minrk

I would like to see how far it can / should be integrated into pyzmq. I think both connect and bind should be able to accept it, what about last_endpoint?

Kentzo avatar Nov 08 '17 19:11 Kentzo

I don't think we should change the return type of any of the existing methods, but accepting these objects in bind/connect makes sense.

minrk avatar Nov 08 '17 21:11 minrk

Perhaps both Address and Endpoint should subclass UserString? Current __iter__ can be replaced with the as_tuple method.

Kentzo avatar Nov 13 '17 19:11 Kentzo

I think that specifically, no base pyzmq APIs should return anything but basic types. Providing convenient wrappers is okay, but I wouldn't change the types, even if they are subclasses that satisfy isinstance

minrk avatar Nov 14 '17 09:11 minrk

@minrk Ok, but considering that, would it be better to have them as strings or objects?

Kentzo avatar Nov 14 '17 18:11 Kentzo

Objects, I think.

minrk avatar Nov 15 '17 09:11 minrk

UserString would allow to avoid any changes to pyzmq though. Which makes sense, as URL introspection does not make sense there. Not with current API.

Kentzo avatar Nov 15 '17 21:11 Kentzo