Warning: Can't synchronize with repository "(default)" (/home/git/ome.git does not appear to be a Git repository.). Look in the Trac log for more information.
Notice: In order to edit this ticket you need to be either: a Product Owner, The owner or the reporter of the ticket, or, in case of a Task not yet assigned, a team_member"

Task #12342 (closed)

Opened 8 years ago

Closed 6 years ago

RFE: Simple gateway delete method

Reported by: spli Owned by: wmoore
Priority: major Milestone: 5.x
Component: General Version: 5.0.2
Keywords: n.a. Cc: python-team@…, java@…
Resources: n.a. Referenced By: n.a.
References: n.a. Remaining Time: n.a.
Sprint: n.a.

Description

I just brought down gretzky because I forgot conn.deleteObjects() doesn't block, and instead returns a handle which you need to wait on and close. A option to block would be nice.

Josh also suggested improving the docs to say you must close the delete handle (though I didn't actually read them so it wouldn't have helped in this case).

Change History (22)

comment:1 Changed 8 years ago by jamoore

  • Cc java@… added
  • Milestone changed from Unscheduled to 5.0.3
  • Priority changed from minor to major

Along with the rest of these fixes, it might make sense to add a server-side limit: "No more than X command objects at a time."

comment:2 Changed 7 years ago by mtbcarroll

How would we tell how many command objects there are at the present time: how do we know when they're done? Is this about tracking in Executor.Impl for how many submissions Future.isDone() == false or am I on the wrong track?

comment:3 Changed 7 years ago by jamoore

The number that is in the executor at any one time is limited, which I had originally thought would be enough. We have throttling components which check how many servants you create and eventually yells loudly if there are too many. We could and should do the same then for the finished but unclosed cmd objects.

comment:4 Changed 7 years ago by mtbcarroll

Thank you. SessionI is looking relevant.

comment:6 Changed 7 years ago by mtbcarroll

When we submit a Request to SessionI it registers a servant delegate tie via the put and remove in ServantHolder that does the overusage check. There is a IHandle mixed in with all this that happens to be an AbstractCloseableAmdServant (which isn't the only kind of CloseableServant that exists).

Perhaps we can build the additional desired check also into ServantHolder. One thing that isn't immediately clear to me is how we tell which servants are even closed, and perhaps they might be closed yet still registered. Might be worth a discussion to go over the general picture of how all this works and precisely what to target.

comment:8 Changed 7 years ago by jburel

  • Milestone changed from 5.1.0-m3 to 5.1.0-m4

To be reviewed as part of mark's work

comment:9 Changed 7 years ago by mtbcarroll

I would certainly be glad for anyone else to pick this up if it's scheduled for a near-term milestone. I wonder if in general the async callback methods should all be offering an alternative blocking option or if we just need some gateway utility method that does the waiting and blocking given the handle?

comment:10 Changed 7 years ago by jamoore

Re: the method itself -- definitely easy enough for someone else to pick it up, Mark. Major question is if the new omero::cmd::Delete object will support *every* type or only those explicitly defined?

There are gateway methods which block in each of the languages. I'd prefer not to have a blocking *remote* method, though.

comment:11 Changed 7 years ago by omero

It'll at least have a go at dealing with any type, though at least some of them need some special rules adding because of some peculiarity or other, often to do with where to start or stop traversing the graph. For instance, given a Well, do also delete its WellSamples (even though there's only a "backlink" in the DB), or, given an Image, delete the Rois and Thumbnails even if they're somebody else's, but don't delete a Channel if some other Image shares it that isn't being deleted.

(Sorry, the above was me, Mark, didn't notice I was logged in as omero!)

Last edited 7 years ago by omero (previous) (diff)

comment:12 Changed 7 years ago by jamoore

  • Owner set to wmoore

Anything here for 5.1.0?

comment:13 Changed 7 years ago by wmoore

This is the first I've seen of this ticket and I'm not sure what's being asked for. Something in Blitz Gateway?

comment:14 follow-up: Changed 7 years ago by jburel

Note that the "deleteObjects"method has been deprecated.

comment:15 Changed 7 years ago by wmoore

Do we want conn.deleteObjects() to have an option to wait?

Then you could do:

def deleteObjects(...wait=False)

if wait:
    try:
        self._waitOnCmd(handle)
    finally:
        handle.close()
return handle

comment:16 Changed 7 years ago by spli

I vote yes

comment:17 in reply to: ↑ 14 Changed 7 years ago by cblackburn

Replying to jburel:

Note that the "deleteObjects"method has been deprecated.

The method isn't deprecated as such but it did use deprecated server commands. These are now fixed in https://github.com/openmicroscopy/openmicroscopy/pull/3739

comment:18 Changed 7 years ago by cblackburn

Having just worked on the gateway, I'd agree on having deleteObjects have a wait option unless there is some really good reason it never did.

comment:19 Changed 7 years ago by wmoore

  • Milestone changed from 5.1.2 to 5.1.3

comment:20 Changed 6 years ago by jamoore

  • Milestone changed from 5.1.4 to OMERO-5.1.4

Splitting 5.1.4 due to milestone decoupling

comment:21 Changed 6 years ago by sbesson

  • Milestone changed from OMERO-5.1.4 to 5.x

As discussed with Will earlier today, pushing the non-critical Web tickets out of 5.1.4

comment:22 Changed 6 years ago by cblackburn

  • Resolution set to fixed
  • Status changed from new to closed

https://github.com/openmicroscopy/openmicroscopy/pull/4266 adds both a wait option to deleteObjects() and a simple blocking deleteObject(). I think that fixes this ticket. Please re-open if I've missed something.

Note: See TracTickets for help on using tickets. You may also have a look at Agilo extensions to the ticket.

1.3.13-PRO © 2008-2011 Agilo Software all rights reserved (this page was served in: 0.168785 sec.)

We're Hiring!