Edgewall Software
Modify

Opened 3 months ago

Closed 6 weeks ago

Last modified 6 weeks ago

#13165 closed defect (fixed)

tracd incorrectly says "needs to be upgraded" if an exception is raised from needs_upgrade()

Reported by: Jun Omae Owned by: Jun Omae
Priority: normal Milestone: 1.3.4
Component: general Version: 1.3dev
Severity: normal Keywords:
Cc: Branch:
Release Notes:

Fix "Environment needs to be upgraded" when an exception is raised from plugin.

API Changes:

Description

Environment.needs_upgrade() raised a TypeError because installed plugin has incompatible method with Trac 1.3.x:

TypeError: environment_needs_upgrade() takes exactly 2 arguments (1 given)

When the error is raised, I get "needs to be upgraded" from tracd. It's incorrect, because the Trac environment is up-to-date.

Trac Error

TracError: The Trac Environment needs to be upgraded. Run:

  trac-admin "/home/jun66j5/var/trac/1.3-sqlite" upgrade

It seems the behavior is introduced in [15939] (#12680).

I consider that we should raise a TracError with Unable to check for upgrade message when an exception is raised from needs_upgrade():

  • trac/env.py

    diff --git a/trac/env.py b/trac/env.py
    index bf3fbb11c..0a6b868a9 100644
    a b def open_environment(env_path=None, use_cache=False):  
    857857                CacheManager(env).reset_metadata()
    858858    else:
    859859        env = Environment(env_path)
    860         needs_upgrade = True
    861860        try:
    862861            needs_upgrade = env.needs_upgrade()
     862        except TracError as e:
     863            env.log.error("Exception caught while checking for upgrade: %s",
     864                          exception_to_unicode(e))
     865            raise
    863866        except Exception as e:  # e.g. no database connection
    864867            env.log.error("Exception caught while checking for upgrade: %s",
    865868                          exception_to_unicode(e, traceback=True))
    866         if needs_upgrade:
    867             raise TracError(_('The Trac Environment needs to be upgraded. '
    868                               'Run:\n\n  trac-admin "%(path)s" upgrade',
    869                               path=env_path))
     869            raise TracError(_("Unable to check for upgrade: %(err)s",
     870                              err=exception_to_unicode(e)))
     871        else:
     872            if needs_upgrade:
     873                raise TracError(_('The Trac Environment needs to be upgraded. '
     874                                  'Run:\n\n  trac-admin "%(path)s" upgrade',
     875                                  path=env_path))
    870876
    871877    return env
    872878

After the patch, tracd says in this case:

Trac Error

TracError: Unable to check for upgrade: TypeError: environment_needs_upgrade() takes exactly 2 arguments (1 given)}

Attachments (0)

Change History (5)

comment:1 by Christian Boos, 3 months ago

Good idea, but reporting which plugin raised such an error seems also very useful in this case (so that one could disable it if needed).

comment:2 by Jun Omae, 3 months ago

Agreed. I reworked the patch based on your suggestion.

Revised patch:

  • trac/env.py

    diff --git a/trac/env.py b/trac/env.py
    index bf3fbb11c..865c93def 100644
    a b class Environment(Component, ComponentManager):  
    745745    def needs_upgrade(self):
    746746        """Return whether the environment needs to be upgraded."""
    747747        for participant in self.setup_participants:
    748             with self.component_guard(participant, reraise=True):
    749                 if participant.environment_needs_upgrade():
    750                     self.log.warning(
    751                         "Component %s requires environment upgrade",
    752                         participant)
    753                     return True
     748            try:
     749                with self.component_guard(participant, reraise=True):
     750                    if participant.environment_needs_upgrade():
     751                        self.log.warning(
     752                            "Component %s requires environment upgrade",
     753                            participant)
     754                        return True
     755            except Exception as e:
     756                raise TracError(_("Unable to check for upgrade of "
     757                                  "%(module)s.%(name)s: %(err)s",
     758                                  module=participant.__class__.__module__,
     759                                  name=participant.__class__.__name__,
     760                                  err=exception_to_unicode(e)))
    754761        return False
    755762
    756763    def upgrade(self, backup=False, backup_dest=None):
    def open_environment(env_path=None, use_cache=False):  
    857864                CacheManager(env).reset_metadata()
    858865    else:
    859866        env = Environment(env_path)
    860         needs_upgrade = True
    861867        try:
    862868            needs_upgrade = env.needs_upgrade()
     869        except TracError as e:
     870            env.log.error("Exception caught while checking for upgrade: %s",
     871                          exception_to_unicode(e))
     872            raise
    863873        except Exception as e:  # e.g. no database connection
    864874            env.log.error("Exception caught while checking for upgrade: %s",
    865875                          exception_to_unicode(e, traceback=True))
    866         if needs_upgrade:
    867             raise TracError(_('The Trac Environment needs to be upgraded. '
    868                               'Run:\n\n  trac-admin "%(path)s" upgrade',
    869                               path=env_path))
     876            raise
     877        else:
     878            if needs_upgrade:
     879                raise TracError(_('The Trac Environment needs to be upgraded. '
     880                                  'Run:\n\n  trac-admin "%(path)s" upgrade',
     881                                  path=env_path))
    870882
    871883    return env
    872884

tracd message after the revised patch:

Trac Error

TracError: Unable to check for upgrade of tracdragdrop.web_ui.TracDragDropModule: TypeError: environment_needs_upgrade() takes exactly 2 arguments (1 given)

Logging after the revised patch:

2019-05-28 21:45:16,920 Trac[env] ERROR: Component <Component tracdragdrop.web_ui.TracDragDropModule> failed with
Traceback (most recent call last):
  File "/home/jun66j5/src/tracdev/git/trac/env.py", line 387, in component_guard
    yield
  File "/home/jun66j5/src/tracdev/git/trac/env.py", line 750, in needs_upgrade
    if participant.environment_needs_upgrade():
TypeError: environment_needs_upgrade() takes exactly 2 arguments (1 given)
2019-05-28 21:45:16,924 Trac[env] ERROR: Exception caught while checking for upgrade: TracError: Unable to check for upgrade of tracdragdrop.web_ui.TracDragDropModule: TypeError: environment_needs_upgrade() takes exactly 2 arguments (1 given)

comment:3 by Ryan J Ollos, 3 months ago

Looks like a good change.

comment:4 by Jun Omae, 6 weeks ago

Release Notes: modified (diff)
Resolution: fixed
Status: newclosed

Thanks. Applied the patch in comment:2 in [16953].

comment:5 by Jun Omae, 6 weeks ago

Owner: set to Jun Omae

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.