#227 New method Compose.transition
Merged 7 months ago by jkaluza. Opened 8 months ago by cqi.

file modified
+20 -30

@@ -170,17 +170,14 @@ 

          composes = Compose.composes_to_expire()

          for compose in composes:

              log.info("%r: Removing compose", compose)

-             compose.state = COMPOSE_STATES["removed"]

              if compose.removed_by:

                  state_reason = "Removed by {}.".format(compose.removed_by)

              else:

                  state_reason = "Compose is expired."

              if compose.state_reason:

-                 compose.state_reason += "\n%s" % state_reason

-             else:

-                 compose.state_reason = state_reason

-             compose.time_removed = datetime.utcnow()

-             db.session.commit()

+                 state_reason = '{}\n{}'.format(compose.state_reason,

+                                                state_reason)

+             compose.transition(COMPOSE_STATES["removed"], state_reason)

              if not compose.reused_id:

                  self._remove_compose_dir(compose.toplevel_dir)

  

@@ -428,6 +425,10 @@ 

      # Set the time_to_expire to bigger value from both composes.

      compose.time_to_expire = max(compose.time_to_expire,

                                   compose_to_reuse.time_to_expire)

+     # NOTE: reuse_compose is only called by generate_pungi_compose at this

+     # moment. This change will be committed when compose state is transitted,

+     # which will call session's commit. If this method is called from somewhere

+     # else in the future, be careful to manage the commit.

      compose_to_reuse.time_to_expire = compose.time_to_expire

  

  

@@ -501,12 +502,9 @@ 

  

      compose.arches = " ".join(arches)

      compose.sigkeys = " ".join(sigkeys)

-     compose.state = COMPOSE_STATES["done"]

-     compose.state_reason = "Compose is generated successfully"

+     compose.transition(COMPOSE_STATES["done"],

+                        "Compose is generated successfully")

      log.info("%r: Compose done", compose)

-     compose.time_done = datetime.utcnow()

-     db.session.add(compose)

-     db.session.commit()

  

  

  def generate_pungi_compose(compose):

@@ -564,12 +562,9 @@ 

      # If there is no exception generated by the pungi.run() and if

      # validation didn't fail, then we know the compose has been

      # successfully generated.

-     compose.state = COMPOSE_STATES["done"]

-     compose.state_reason = "Compose is generated successfully"

+     compose.transition(COMPOSE_STATES["done"],

+                        "Compose is generated successfully")

      log.info("%r: Compose done", compose)

-     compose.time_done = datetime.utcnow()

-     db.session.add(compose)

-     db.session.commit()

  

      koji_tag_cache.update_cache(compose)

  

@@ -629,15 +624,12 @@ 

                  log.exception("%r: Error while generating compose", compose)

              else:

                  log.exception("Error while generating compose %d", compose_id)

-             compose.state = COMPOSE_STATES["failed"]

-             compose.time_done = datetime.utcnow()

  

              pungi_logs = PungiLogs(compose)

-             compose.state_reason = "Error while generating compose: %s\n" % str(e)

-             compose.state_reason += pungi_logs.get_error_string()

+             state_reason = "Error while generating compose: {}\n{}".format(

+                 str(e), pungi_logs.get_error_string())

  

-             db.session.add(compose)

-             db.session.commit()

+             compose.transition(COMPOSE_STATES["failed"], state_reason)

  

          compose = Compose.query.filter(Compose.id == compose_id).one()

  

@@ -671,10 +663,9 @@ 

          Adds the compose to queue of composes to generate, so

          the ThreadPoolExecutor can start working on it.

          """

-         compose.state = COMPOSE_STATES["generating"]

-         compose.state_reason = "Compose thread started"

-         db.session.add(compose)

-         db.session.commit()

+         compose.transition(COMPOSE_STATES["generating"],

+                            "Compose thread started")

+ 

          self.currently_generating.append(compose.id)

          if compose.source_type == PungiSourceType.PULP:

              self.pulp_executor.submit(generate_compose, compose.id)

@@ -727,9 +718,9 @@ 

  

          for compose in composes:

              if compose.time_submitted < from_time:

-                 compose.state = COMPOSE_STATES["failed"]

-                 compose.state_reason = "Compose stuck in 'wait' state for longer than 3 days."

-                 db.session.add(compose)

+                 compose.transition(

+                     COMPOSE_STATES["failed"],

+                     "Compose stuck in 'wait' state for longer than 3 days.")

                  continue

  

              # Take only num_concurrent_pungi * 2 non-Pulp composes to keep some queue

@@ -742,7 +733,6 @@ 

              log.info("%r: Going to regenerate compose stuck in 'wait' "

                       "state.", compose)

              self.generate_new_compose(compose)

-         db.session.commit()

  

      def generate_lost_composes(self):

          """

@@ -337,5 +337,21 @@ 

          if new_expiration != self.time_to_expire:

              self.time_to_expire = new_expiration

  

+     def transition(self, to_state, reason, happen_on=None):

+         """Transit compose state to a new state

+ 

+         :param str to_state: transit this compose state to this state.

+         :param str reason: the reason of this transition.

+         :param happen_on: when this transition happens. Default is utcnow.

+         :type happen_on: DateTime

+         """

+         self.state = to_state

+         self.state_reason = reason

+         if to_state == COMPOSE_STATES['removed']:

+             self.time_removed = happen_on or datetime.utcnow()

+         elif to_state == COMPOSE_STATES['done']:

+             self.time_done = happen_on or datetime.utcnow()

+         db.session.commit()

+ 

  

  Index('idx_source_type__state', Compose.source_type, Compose.state)

A comopse state transits several times in backend from wait to done and other
finish state. This patch introduce a new method Compose.transition to
change a compose' state with reason instead of repeating lines of code
to change state, reason, corresponding time and commit.

Signed-off-by: Chenxiong Qi cqi@redhat.com

I'm thinking of to reimplement to send UMB message based on this when compose state is changed. It should be easier than original implementation by using SQLAlchemy signal.

Pull-Request has been merged by jkaluza

7 months ago