Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-13672

Make AssociationTask stateless

    Details

    • 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

            Hide
            rowen Russell Owen added a comment -

            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.

            Show
            rowen Russell Owen added a comment - 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.
            Hide
            krzys 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...

            Show
            krzys 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...
            Hide
            rowen Russell Owen added a comment -

            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 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.

            Show
            rowen Russell Owen added a comment - 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 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.
            Hide
            krzys 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.

            Show
            krzys 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.
            Hide
            rowen Russell Owen added a comment - - edited

            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.

            Show
            rowen Russell Owen added a comment - - edited 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.

              People

              • Assignee:
                Unassigned
                Reporter:
                krzys Krzysztof Findeisen
                Watchers:
                Krzysztof Findeisen, Russell Owen
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:

                  Summary Panel