trac-admin "changeset modified" with an invalid changeset is not logged

[Ryan J Ollos]

With the changes implemented in #11386, trac-admin $env changeset added $repos $rev with an invalid rev results in an error being logged. However an error is not logged when the changeset modified command is called.

The following is the output with logging directed to the console:

$ trac-admin ../tracdev changeset modified tracdev2 100
NoSuchChangeset: No changeset 100 in the repository
$ trac-admin ../tracdev changeset added tracdev2 100
07:32:46 Trac[api] DEBUG: No changeset '100' found in repository 'tracdev2'. Skipping subscribers for event changeset_added

[Ryan J Ollos]

Proposed changes in log:rjollos.git:t11775. It might be good to add a unit test or two before pushing the changes.

[Jun Omae]

I reconsider about Skipping subscribers … messages, perhaps we should use log.warn() rather than log.debug() when no such changeset because the specified reponame and/or revs are probably wrong…?

trac/versioncontrol/api.py

@@ -693 +693 @@
                     try:
                         old_changeset = repos.sync_changeset(rev)
                     except NoSuchChangeset:
-                        self.log.debug(
+                        self.log.warn(
                             "No changeset '%s' found in repository '%s'. "
                             "Skipping subscribers for event %s",
                             rev, reponame, event)
@@ -707 +707 @@
                         repos.sync_changeset(rev)
                         changeset = repos.get_changeset(rev)
                     except NoSuchChangeset:
-                        self.log.debug(
+                        self.log.warn(
                             "No changeset '%s' found in repository '%s'. "
                             "Skipping subscribers for event %s",
                             rev, reponame, event)

[Ryan J Ollos]

Logging a warning seems like I good idea. I'll include the comment:2 patch in the changes.

[Ryan J Ollos]

Replying to jomae:

I reconsider about Skipping subscribers … messages, perhaps we should use log.warn() rather than log.debug() when no such changeset because the specified reponame and/or revs are probably wrong…?

Log level for existing message changed in [13339], merged in [13340].

[Ryan J Ollos]

Revised changes in log:rjollos.git:t11775.2.

$ trac-admin ../tracdev changeset added "(default)" 100 101
NoSuchChangeset: No changeset 100 in the repository
NoSuchChangeset: No changeset 101 in the repository

$ trac-admin ../tracdev changeset modified "(default)" 100 101
NoSuchChangeset: No changeset 100 in the repository
NoSuchChangeset: No changeset 101 in the repository

[Jun Omae]

Revised changes in log:rjollos.git:t11775.2.

That changes sound good idea.

But, I got syntax errors with Python 2.5.

  File "/run/shm/2dd50c3e4d700d0f2689f9b2b7e1bcb25171b202/py25-sqlite/trac/versioncontrol/api.py", line 697
    except NoSuchChangeset as e:
                            ^
SyntaxError: invalid syntax
make: *** [unit-test] Error 1

We should log with WARN level rather than DEBUG level in changeset_modified event when raising a NoSuchChangeset exception?

[Ryan J Ollos]

Thanks for catching those two issues. I also fixed use of outer exception object when inner exception object should be created and used. It is confusing with the nesting. Committed to 1.0-stable in [13341], merged to trunk and , e → as e in [13342].

[Ryan J Ollos]

Related issue in #12665.