Details
-
Type:
Improvement
-
Status: Invalid
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: ap_association
-
Labels:
-
Team:Alert Production
Description
Currently AssociationTask is associated with an open DB handle, which must be externally closed once the object is no longer in use. This is a problem in the long term because stateful tasks are not supported by the tasks framework, and will be forbidden once SuperTask is released.
DB handles may need to be associated with a call to run rather than with an AssociationTask instance, although I hope there's a better design that I missed.
Attachments
Issue Links
Activity
Oops, I completely forgot about this comment, and it directly addresses an assumption I made in DM-13163. Why do you say the butler is normally passed to run? Looking at ProcessCcdTask, ImageDifferenceTask, ButlerInitializedTaskRunner, etc. I get the opposite impression, that a task object is associated at construction time with specific input/output repositories. Plus it seems strange to allow different datasets to live in different repositories...
Krzysztof Findeisen
added a comment - - edited Oops, I completely forgot about this comment, and it directly addresses an assumption I made in DM-13163 . Why do you say the butler is normally passed to run ? Looking at ProcessCcdTask , ImageDifferenceTask , ButlerInitializedTaskRunner , etc. I get the opposite impression, that a task object is associated at construction time with specific input/output repositories. Plus it seems strange to allow different datasets to live in different repositories... A butler is normally passed to runDataRef indirectly as part of data refs (though a TaskRunner can be modified to pass the butler directly). (Originally the method was called run, which is why I wrote that). I'm not actually sure if the current butler does a good job with database access, but I thought it worked. Kian-Tat Lim or Nate Pease [X] would be able to say much better than I. In any case if the butler is not presently a suitable way of communicating with your database, then using the task's TaskRunner to pass a db handle directly to runDataRef or some similarly-named method seems like a good way to go.
So I think the answer to my question is "butler constructor arguments exist in old tasks but are not encouraged in new ones". That does clarify things.
Krzysztof Findeisen
added a comment - So I think the answer to my question is "butler constructor arguments exist in old tasks but are not encouraged in new ones". That does clarify things. ProcessCcdTask is a modern task. The only reason a butler is passed to the constructor is explained in its docs:
@param[in] butler The butler is passed to the refObjLoader constructor in case it is
|
needed. Ignored if the refObjLoader argument provides a loader directly.
|
I consider it a wart that we need to do this, but we so far have not figured out a way around it.
As far as I know, this is the only reason a butler is ever properly passed to a task constructor.
Closing as Invalid, since this is not the case in the Gen 3 version of AssociationTask.
database access is supposed to be via the butler, and the butler would normally be passed in via the run method. I am guessing in this case the butler cannot write to the database (perhaps due to
DM-11767). If so, passing the db handle to run is very likely better than keeping it a state variable. I have not seen how much violence that does to the code, but if it just means passing the handle from run to other methods then that's a normal pattern.