project-receptor / python-receptor

Project Receptor is a flexible multi-service relayer with remote execution and orchestration capabilities linking controllers with executors across a mesh of nodes.
Other
32 stars 21 forks source link

`--listen` parameter lets one specify nonsensical behaviour #153

Closed Ichimonji10 closed 4 years ago

Ichimonji10 commented 4 years ago

I can start receptor like so:

poetry run receptor --data-dir="$(mktemp --directory)" node --listen=receptor://127.0.0.1:8888

This works fine. But let's say I start receptor like so:

poetry run receptor --data-dir="$(mktemp --directory)" node --listen=receptor://127.0.0.1:8888/foo

In this case, receptor will (to the best of my knowledge) silently discard the path /foo. This seems problematic to me. The user might expect one thing ("receptor will nest its listen interface under the /foo path"), but receptor will do something else. This could be a source of user confusion.

If receptor is going to discard the path, then why does it allow a path to be specified in the first place? Perhaps receptor should reject --listen arguments with information that'd be dropped anyway. For example, the following URL could be rejected:

>>> from urllib.parse import urlparse
>>> urlparse('receptor://127.0.0.1/0:8888')
ParseResult(scheme='receptor', netloc='127.0.0.1', path='/0:8888', params='', query='', fragment='')

The logic could be something like:

if res.path or res.params or res.query or res.fragment:
    raise InvalidListenArgumentError(...)
Ichimonji10 commented 4 years ago

Alternately, receptor could provide parameters like --listen-scheme, --listen-address and --listen-port. That'd be a much more verbose option, though, and it doesn't strike me as being as elegant.

ghjm commented 4 years ago

I can get behind the idea of making it an error to provide path information in the URL that won't be used. That would deal with the receptor://0.0.0.0/0:8888 case.

Does path information possibly mean something in the case of a websocket listener?

Ichimonji10 commented 4 years ago

Does path information possibly mean something in the case of a websocket listener?

Good question. I've no idea. :woman_shrugging: @matburt ?

Ichimonji10 commented 4 years ago

Also worth thinking about: can we tell receptor to bind to an interface, and let it discover that interface' address at runtime? That functionality is both common and useful, and it might affect how this issue is addressed.

Ichimonji10 commented 4 years ago

Fixed by #157. Automated tests added in #139.