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.

Sign up for GitHub

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

Jump to bottom

Move gateway calls to OMEROGateway #305

Merged
merged 4 commits into from
Nov 10, 2022
Merged

Move gateway calls to OMEROGateway #305

merged 4 commits into from
Nov 10, 2022

Conversation

dominikl
Copy link
Member

@dominikl dominikl commented May 27, 2022 •
edited
Loading

Should fix most cases which trigger #99 . This PR moves some Java Gateway calls into the OMEROGateway, because there connection issues can be dealt with by trying to reconnect.

Test: Open Insight, login, browse to a dataset with some images, click on one image. Disconnect from the network. Click on another image. Without the PR: Crash with connection exception. With the PR: Reconnection dialog will pop up. Connect to the network again. After a few seconds Insight should have recovered.

Note: I made the OMEROGateway class public for convenience, otherwise I'd have to add another few delegation methods to the OMERODataservice class etc.

@snoopycrimecop
Copy link
Member

Conflicting PR. Removed from build OMERO-insight-push#1147. See the console output for more details.
Possible conflicts:

  • PR Remove joinSession #296 dominikl 'Remove joinSession'
    • src/main/java/org/openmicroscopy/shoola/env/data/DataServicesFactory.java

--conflicts

@dominikl
Copy link
Member Author

dominikl commented Jun 1, 2022

That also seems to happen somewhere in the ThumbnailLoader: https://www.openmicroscopy.org/qa2/qa/feedback/31109/ .

@snoopycrimecop
Copy link
Member

Conflicting PR. Removed from build OMERO-insight-push#1152. See the console output for more details.
Possible conflicts:

  • PR Remove joinSession #296 dominikl 'Remove joinSession'
    • src/main/java/org/openmicroscopy/shoola/env/data/DataServicesFactory.java

--conflicts

jburel
jburel reviewed Jun 2, 2022
View reviewed changes
src/main/java/org/openmicroscopy/shoola/env/data/OMEROGateway.java Outdated
@@ -256,7 +258,7 @@
* @since OME2.2
*/
@SuppressWarnings({"rawtypes", "unchecked"})
class OMEROGateway
public class OMEROGateway
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a major change.
Methods in the gateway are accessed via the various API/implementation classes

jburel
jburel reviewed Jun 2, 2022
View reviewed changes
src/main/java/org/openmicroscopy/shoola/env/data/views/calls/HistogramLoader.java Outdated
result = f.getHistogram(ctx, img.getDefaultPixels(),
channels, z, t);
}
result = context.getDataService().getOMEROGateway().getHistogram(ctx, img.getDefaultPixels(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should add a method to one of the I/F instead of making the class public

jburel
jburel reviewed Jun 2, 2022
View reviewed changes
src/main/java/org/openmicroscopy/shoola/env/data/views/calls/ROICountLoader.java Outdated
@@ -46,8 +46,7 @@ private BatchCall makeLoadCalls(final SecurityContext ctx,
final long imageID) {
return new BatchCall("Load ROI count from Server") {
public void doCall() throws Exception {
results = context.getGateway().getFacility(ROIFacility.class)
.getROICount(ctx, imageID);
results = context.getDataService().getOMEROGateway().getROICount(ctx, imageID);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

@snoopycrimecop
Copy link
Member

Conflicting PR. Removed from build OMERO-insight-push#1153. See the console output for more details.
Possible conflicts:

  • PR Remove joinSession #296 dominikl 'Remove joinSession'
    • src/main/java/org/openmicroscopy/shoola/env/data/DataServicesFactory.java

--conflicts

@snoopycrimecop
Copy link
Member

Conflicting PR. Removed from build OMERO-insight-push#1161. See the console output for more details.
Possible conflicts:

  • PR Remove joinSession #296 dominikl 'Remove joinSession'
    • src/main/java/org/openmicroscopy/shoola/env/data/DataServicesFactory.java

--conflicts

@snoopycrimecop
Copy link
Member

snoopycrimecop commented Jul 16, 2022 •
edited
Loading

Conflicting PR. Removed from build OMERO-insight-push#1201. See the console output for more details.
Possible conflicts:

  • PR Remove joinSession #296 dominikl 'Remove joinSession'
    • src/main/java/org/openmicroscopy/shoola/env/data/DataServicesFactory.java

--conflicts Conflict resolved in build OMERO-insight-push#1206. See the console output for more details.

@dominikl dominikl mentioned this pull request Jul 21, 2022
Merged
@dominikl dominikl force-pushed the connection_issues branch from 49f413a to d5f4d8b Compare July 21, 2022 14:53
@snoopycrimecop snoopycrimecop mentioned this pull request Jul 22, 2022
Open
@dominikl
Copy link
Member Author

Is this one good to merge @jburel ? Or shall I exclude it for now? Trying to get my conflicting PRs sorted...

@jburel
Copy link
Member

jburel commented Jul 22, 2022 •
edited
Loading

@dominikl sorry I did not notice the new commits
I don't think the gateway should be made public

@dominikl
Copy link
Member Author

Ah yes, sorry, that method is actually not necessary anymore, removed it.

@dominikl dominikl closed this Jul 22, 2022
@dominikl dominikl reopened this Jul 22, 2022
@dominikl
Copy link
Member Author

It would be good to get this in. This would fix a lot of these kinds of crashes on temporary nework loss:

java.lang.Exception: omero.gateway.exception.DSOutOfServiceException: Connection lost.
	at omero.gateway.facility.Facility.handleException(Facility.java:288)
	at omero.gateway.facility.ROIFacility.loadROIs(ROIFacility.java:315)

I'll give it another test and put it up for review again...

@jburel
Copy link
Member

jburel commented Nov 10, 2022

Thanks @dominikl
sorry for not looking at it before.
Merging so we can review the OME-Zarr import PR

@jburel jburel merged commit 420cf06 into ome:master Nov 10, 2022
@dominikl dominikl mentioned this pull request Jan 20, 2023
Closed
@dominikl dominikl mentioned this pull request Mar 6, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jburel jburel jburel left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dominikl @snoopycrimecop @jburel