search

ome / omero-web

Django-based OMERO.web client
https://www.openmicroscopy.org/omero
16 stars 29 forks source link

get strings from omero.cmd.ERR and omero.CmdError correctly #534

Open will-moore opened 4 months ago

will-moore commented 4 months ago

Fixes #481

This fixes 2 similar bugs where we're handling omero.CmdError or omero.cmd.ERR objects and converting into strings.

Both were previously failing with AttributeError: 'ERR' object has no attribute 'message'.

Changes based on using the appropriate attributes as tested at https://github.com/ome/omero-web/issues/481#issuecomment-1959903979

will-moore commented 4 months ago

I tried str(err) for both cases at https://github.com/ome/omero-web/issues/481#issuecomment-1959903979 and it gave sub-optimal results. Not sure exactly what the difference is between omero.CmdError and omero.cmd.ERR!

will-moore commented 4 months ago

@sbesson updated as suggested to use str(omero.CmdError) and str(omero.cmd.ERR.name)

will-moore commented 3 months ago

cc @knabar This is not an urgent PR, but it is a trivial fix so could maybe get reviewed and included in the next release?

sbesson commented 3 months ago

@will-moore I have not had a chance to get back to those. For both updates, do you have scenarios that would allow to reproduce the original errors and asses the impact of the changes functionally?

will-moore commented 3 months ago

@sbesson No, unfortunately I don't have scenarios to reproduce those errors. We only have stack traces from QA, but don't know what caused them (difficult to understand from the stack traces themselves). I could try guessing a few, but not sure where to start

knabar commented 3 months ago

@will-moore I'm curious if these missing attributes existed in the past, and if so, when they were removed or renamed. Discussed with @sbesson a bit and he pointed out this could be related to Python 2/3 upgrades as well.

Overall I feel we need a bit more investigation, so I'd like to hold off on this one until 5.26.0, which could come fairly soon anyway.

will-moore commented 3 months ago

@knabar I don't think they've ever existed. It was just assumed that an exception would have a message attribute.