-
Notifications
You must be signed in to change notification settings - Fork 30
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
imgData JSON with no stateful services #536
imgData JSON with no stateful services #536
Conversation
d17518c
to
944dd03
Compare
a074c29
to
cc3f741
Compare
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
8bfba4c
to
a3cb187
Compare
Deploy on idr-testing...
This failed since it's still python 3.6 and this branch (based on latest omero-web) requires Django 4.2!
|
Conflicting PR. Removed from build OMERO-python-superbuild-push#470. See the console output for more details.
--conflicts |
EDIT - svs issue identified here moved to IDR/idr-metadata#691 |
Conflicting PR. Removed from build OMERO-python-superbuild-push#477. See the console output for more details.
|
…t_stateful_services
There are still some concerns about this potentially introducing breaking API changes. Are issues like #536 (comment) still happening? Performance improvements seem to be limited to particular image types (#537 (comment)). The way image pyramids/tiles are detected changes the behavior of the API endpoint.
Both those options would guarantee that no clients break in an unexpected fashion with any images, and clients that might benefit could be updated to use the new behavior with a single line change to the new endpoint or by adding the query argument. |
Sorry - please ignore #536 (comment) - I just happened to find that bug when testing this PR so I reported it here, but it's not related to this PR. I'll revert the API changes as suggested (always use renderingEngine to check for pyramids), then we can discuss if it's worth adding a |
82149cf
to
ff4f516
Compare
@knabar I replaced much of the code in the same order as before, so the PR changes are reduced. |
I set up a temporary branch that exposes both the original and the new Using the following commands on ID=347168
COOKIE='Cookie: csrftoken=...; sessionid=...'
diff <(curl -s http://localhost:4080/webgateway/imgDataOriginal/${ID}/ -H ${COOKIE} | jq --sort-keys .) \
<(curl -s http://localhost:4080/webgateway/imgData/${ID}/ -H ${COOKIE} | jq --sort-keys .) So far the main difference is with the channels
cc: @sbesson |
Re: #536 (comment) float vv ints:
|
For some images where
The |
As discussed in web meeting today, let's test on idr-testing without deploying this PR (via branch at #548) and see if it's really needed for idr0090 or if other improvements are sufficient to provide acceptable performance and stability. Deployed vanilla omero-web with
|
Since it appears we don't now need these changes for IDR, closing.... |
It has been noted in some cases that the Preview panel and the right panel of iviewer can take a long time to load, since we currently initialise various stateful services (which requires initialising them with the original image files). This means that the viewers are slower to load than if we just get the data from the DB, particularly if access to original data is slow.
Stateful services usage is replaced as follows:
image.getPixelRange()
, we lookup values based on pixel typerequiresPixelsPyramid()
we useimage.requiresPixelsPyramid()
which simply uses the size cut-off.To test:
re.lookupRenderingDef(pid, ctx)
for the logic of "use my rdef or use the owners", this is now encoded inwebgateway.util.get_rendering_def()
Also see performance testing with NGFF data on idr-testing at #537 (comment)
Summarised as follows: