6 Replies Latest reply on Mar 2, 2009 4:02 AM by thomas.diesler

    Removed Timer/autosave in Revision 4006?

    camunda

       

      Bernd wrote:

      > Hi Alex!
      > With Revision 4006 you removed the fix for
      > https://jira.jboss.org/jira/browse/JBPM-1015 from the Timer class.
      > basically this code:
      > Was this by accident? Or what rational was behind this? Can I
      > re-include it?
      > Because now, Bug 1015 would occur again with jbpm 3.2.6


      Alejandro wrote:

      While working on JBPM-2036, I noticed that the addAutoSaveToken call was
      duplicated in all job classes, so I moved it to JobExecutorThread. Now
      that you mention it, the code should be added to ExecuteJobCommand as
      well.

      Since no test failed, I assumed the net effect was the same. I guess it
      broke something on your side. Care to tell me how to reproduce?


      Hi Alex.

      Couldn't we move this to the JobSession.loadJob() method? Now we have to spread this logic to JobExecutor, ExecuteJobCommand's and so on.

      In my case, we have an own ExecuteJobCommand, since we do some additional stuff. So I had to change that as well to get the Tests green. Maybe other people do the same stuff?

      And in JobSession it would be nice, since then it is "close" to the code handling persistence, or not?

      Remarks?

      @Ronald: Yeah, I see a slight chance, that it was missed to write a test for executing jobs via Command ;-)

      Cheers
      Bernd

      P.S @Alex: I fly to Mexico next week. Maybe we could just meet for beer on around Tuesday 10.03. in Mexico City?

        • 1. Re: Removed Timer/autosave in Revision 4006?
          aguizar

           

          "camunda" wrote:
          In my case, we have an own ExecuteJobCommand, since we do some additional stuff. So I had to change that as well to get the Tests green. Maybe other people do the same stuff?

          Yes, the Seam integration also uses a custom job executor and they may rely on the jobs autosaving the process instance.

          "camunda" wrote:
          Couldn't we move this to the JobSession.loadJob() method? Now we have to spread this logic to JobExecutor, ExecuteJobCommand's and so on.

          Seems like a good proposal, except that loadJob could also be used under contexts that do not require saving the process instance (e.g. a monitoring console). I have the following two proposals:

          1. Introduce a loadJobForUpdate method, possibly in JbpmContext (since no XxxSession has any xxxForUpdate method).
          2. Forget about it, and just revert.

          I'm leaning towards (1). Any thoughts?

          "camunda" wrote:
          I fly to Mexico next week. Maybe we could just meet for beer on around Tuesday 10.03. in Mexico City?

          Crap, what a bad coincidence. Right now I'm in Houston, USA, and will be back to Mexico City by March 14th. Perhaps we could meet on your way back?

          • 2. Re: Removed Timer/autosave in Revision 4006?
            camunda

             

            except that loadJob could also be used under contexts that do not require saving the process instance


            But does this harm or break anything? I always experience a bit confusion of the people about the "forUpdate" methods. And you can forget to use them. Much easier is that is hidden and done automatically...

            But introducing the xxxForUpdate on the JbpmContext seems like the second best option for me (and consistent with other jbpm stuff).

            Perhaps we could meet on your way back?
            .
            Bummer. No, I fly back from Cancun and don't return to Mexico City (only for transit in the plane). Next time ;-)

            • 3. Re: Removed Timer/autosave in Revision 4006?
              aguizar

               

              I always experience a bit confusion of the people about the "forUpdate" methods. And you can forget to use them. Much easier is that is hidden and done automatically...

              Agreed. Something to watch for in jBPM 4. In version 3, that is how things are...
              introducing the xxxForUpdate on the JbpmContext seems like the second best option for me (and consistent with other jbpm stuff).

              In the end I decided against it in 3.2.6, because we are about to release. I'll dig on this a bit more in 3.2.7.

              • 4. Re: Removed Timer/autosave in Revision 4006?
                camunda

                so what is in jbpm 3.2.6?

                • 5. Re: Removed Timer/autosave in Revision 4006?
                  aguizar

                  Each job subclass registers the process instance for autosave, as in your fix for JBPM-1015. I removed the redundant calls to jbpmContext.save() that had been left over.

                  • 6. Re: Removed Timer/autosave in Revision 4006?
                    thomas.diesler

                    Bernd, please clone the issue or create a new one.

                    Generally you cannot commit stuff to an already closed issue - if there is still stuff to do there must be an unresolved issue for it. This is important for documentation purposes in the release notes.

                    To preserve issue association they can be linked to each other.


                    I'll dig on this a bit more in 3.2.7


                    If there is an issue, please create one.