ome / omero-py

Python project containing Ice remoting code for OMERO
https://www.openmicroscopy.org/omero
GNU General Public License v2.0
20 stars 31 forks source link

Informative error reason set to None before sending to logger in clients.py #352

Closed hillan141 closed 1 year ago

hillan141 commented 1 year ago

Thanks for supporting omero.

https://github.com/ome/omero-py/blob/master/src/omero/clients.py#L631

In client.createSession() if one of two Exceptions occurs, an informative reason for the exception is captured. However, the value of reason is set to None before it is logged at https://github.com/ome/omero-py/blob/master/src/omero/clients.py#L633. As a result, in log entries, the reason is always None, losing information about the reason for failure of createSession().

In a recent debugging session of a connection timeout, being able to see the reason in the log would have been very helpful.

Suggestion: move assignment of reason at https://github.com/ome/omero-py/blob/master/src/omero/clients.py#L631 up a line so that it is outside the while() loop.

sbesson commented 1 year ago

@hillan141 thanks for the feedback and agreed. As things stands reason is unnecessarily reset to None on each retry. I opened #353 to propose the change you suggested for inclusion in the next OMERO.py release