neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21504 )

Change subject: add osmo_vty.py
......................................................................


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21504/3/src/osmo_gsm_tester/obj/osmo_vty.py
File src/osmo_gsm_tester/obj/osmo_vty.py:

https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21504/3/src/osmo_gsm_tester/obj/osmo_vty.py@64
PS3, Line 64:             self.sck = None
> is socket closed whenever you set it to None? I'd prefer having an explicitly 
> sck.close() here.
the None is just an indicator that the socket is currently not open.

socket cleanup should be explicit. i guessed sck.close() needs to happen only 
after sck.connect() was successful, and otherwise the socket would be 
implicitly closed from the ConnectionRefusedError.

from python3 docs:

 Sockets are automatically closed when they are garbage-collected, but it is 
recommended to close() them explicitly, or to use a with statement around them.

interesting:

  socket.close()
    Mark the socket closed. The underlying system resource (e.g. a file 
descriptor) is also closed when all file objects from makefile() are closed.

and

 Note
 close() releases the resource associated with a connection but does not 
necessarily close the connection immediately. If you want to close the 
connection in a timely fashion, call shutdown() before close().

i should anyway probably 'except' on anything, not just ConnectionRefusedError.



--
To view, visit https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21504
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-gsm-tester
Gerrit-Branch: master
Gerrit-Change-Id: I7c17b143b7c690b8c4105ee7c6272670046fa91d
Gerrit-Change-Number: 21504
Gerrit-PatchSet: 3
Gerrit-Owner: neels <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-Comment-Date: Mon, 07 Dec 2020 21:45:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <[email protected]>
Gerrit-MessageType: comment

Reply via email to