Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Download_as closes RE #3639

Merged
merged 3 commits into from Mar 20, 2015
Merged

Conversation

will-moore
Copy link
Member

This fixes http://trac.openmicroscopy.org/ome/ticket/12794 which is due to a couple of bugs in webgateway/download_as for batch exporting images as PNG, JPG etc. See https://www.openmicroscopy.org/community/viewtopic.php?f=5&t=7766

We were using @login_required(doConnectionCleanup=False) incorrectly here, since we are not streaming data from OMERO directly (don't need to keep services open once the request returns and streaming starts). This meant that open Rendering Engines were not getting closed.

We were also failing to call image._re.close() for each image after rendering and saving into the zip folder. For each image, a new ProxyObjectWrapper for each rendering engine was getting created but the underlying rendering engines were not getting closed each time.

Both these issues are fixed now.
Need to test that all "Export As" functionality is still working.

Potential backport https://trello.com/c/tgJwB05l/188-5-0-9-potential-backports

--no-rebase

@joshmoore
Copy link
Member

Thanks for this, @will-moore. The only thing I would add for these close statements would be that having them in a try/finally would be double-plus-good.

@jburel jburel added the develop label Mar 19, 2015
@will-moore
Copy link
Member Author

@joshmoore Is that what you meant?

@gusferguson
Copy link

@will-moore

Tested with https://trout.openmicroscopy.org/merge/webclient/ user-3 read-only-1 and user-12 on user-3 data.

Behaves as expected.
Good to merge.

@joshmoore
Copy link
Member

@will-moore : exactly. Thanks. If we could slowly but consistently move to having that everywhere, it'd be great.

joshmoore added a commit that referenced this pull request Mar 20, 2015
@joshmoore joshmoore merged commit d340959 into ome:develop Mar 20, 2015
@joshmoore joshmoore deleted the download_as_close_RE branch March 20, 2015 10:18
@will-moore
Copy link
Member Author

@joshmoore OK. For most web uses we only access an image at a time, so we don't get a ton of open REs, but certainly Batch_Image_Export script and OMERO.figure export script need this.

@will-moore
Copy link
Member Author

PR for Batch_Image_Export: ome/omero-scripts#100

@joshmoore
Copy link
Member

Great, thanks, @will-moore!

@sbesson sbesson added this to the 5.1.0-m5 milestone Mar 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants