search

golemfactory / yapapi

Python high-level API for Golem.
https://yapapi.readthedocs.io/en/stable/
GNU Lesser General Public License v3.0
49 stars 24 forks source link

SocketProxy doesn't check if the port is already in use. #1141

Open cryptobench opened 1 year ago

cryptobench commented 1 year ago

Issue

While progressing with the development of the Golem portal, we stumbled upon an unexpected issue related to Socketproxy's port allocation.

We found that Yapapi permits the allocation of the same port to two instances of an identical script, without issuing any warning or error message. The lack of notification or exception on Yapapi's part was surprising, considering our expectation that it would raise some sort of alert in such situation.

Steps to Replicate:

  1. Open two terminal instances.
  2. In each terminal, run the same SSH example provided in the Yapapi examples.
image
cryptobench commented 1 year ago

Update

The error is printed after shutting down the script, which doesn't seem intended.

image
shadeofblue commented 1 year ago

in this case, it's not really a problem with the library itself but with the code that starts the proxy - we'll update the example but you'd be responsible for updating the script you're using the SocketProxy in...

cryptobench commented 1 year ago

in this case, it's not really a problem with the library itself but with the code that starts the proxy - we'll update the example but you'd be responsible for updating the script you're using the SocketProxy in...

Wouldn't it make more sense to hide away this check from the examples and instead put it inside the internals of yapapi as a type of Exception?

I would personally refrain from us having to add extra code in the examples to check if its in use or not. It's extra "bloat" for the eyes of new users, which could be hidden away.