Conversation
src/omero_cli_render.py
Outdated
| "Updated rendering settings for Image:%s" % img.id) | ||
| if not args.skipthumbs: | ||
| self._generate_thumbs([img]) | ||
| img._closeRE() |
There was a problem hiding this comment.
In general, big 👍 for closing services, but:
- if this needs to be here, probably best in a try/finally
- I wonder though why your
gateway_requiredecorators isn't handling this
There was a problem hiding this comment.
My understanding is that the decorator will handle the closing of the gateway and its services at the end of the command. But I do not see anything here or in the gateway that would close each rendering enging within the loop.
As noticed by @dominikl and myself, this becomes especially important when using bin/omero render set against a large collection of objects e.g. a plate or a screen.
sbesson
left a comment
sbesson
left a comment
There was a problem hiding this comment.
Functionally tested against a plate with 308 wells with 30 fields of view each. Without this PR, bin/omero render set Plate:<id> render.yml fails with an OOM after a number of iterations. With it included, the rendering generation is progressing steadily.
I would concur with #29 (comment). The only addition I would propose would be to have the rendering engine closed in a try/finally block. Once done, I propose we get this merged and tagged as a patch release.
src/omero_cli_render.py
Outdated
| or 0 if it cannot be determined. | ||
| """ | ||
| Returns the version of the rendering settings format. | ||
| Npte: Previously min/max was used to set the channel window start/end |
| img._closeRE() | ||
| try: | ||
| img._closeRE() | ||
| except Exception as e: |
There was a problem hiding this comment.
@will-moore does _closeRE() handle exceptions internally?
In addition, I think the question of #29 (comment) was to handle any exception thrown in the previous calls e.g. in img.saveDefaults() to ensure the service would be closed
try:
...
img.saveDefaults()
...
finally:
img._closeRE()
There was a problem hiding this comment.
How do you actually know that saveDefaults() can throw an exception? I'll still find it unbelievabe difficult to do use the cli and/or BlitzGatway methods. E.g. https://downloads.openmicroscopy.org/omero/5.4.10/api/python/omero/omero.gateway.html#omero.gateway._ImageWrapper.saveDefaults doesn't mention any exception, it doesn't even say anything about what the method actually does.
|
Thanks @dominikl. Merging and releasing as 0.4.3 so that we can start consuming it as we render plates. |
4 participants
setcommand). I wonder if that caused the render plugin to fall over if it was running repeatedly for a long time reusing the same session.copycommand implementation a bit (was unnecessarily split over three different methods).