Edgewall Software
Modify

Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#11775 closed defect (fixed)

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

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.0.3
Component: version control Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:
  • A warnings is logged when an invalid changeset is passed to RepositoryManager.notify().
  • An exception without traceback is printed to the console when TracAdmin's changeset added or changeset modified is called with invalid revision arguments.
API Changes:
Internal Changes:

Description

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

Attachments (0)

Change History (8)

comment:1 by Ryan J Ollos, 6 years ago

Owner: set to Ryan J Ollos
Status: newassigned

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

comment:2 by Jun Omae, 6 years ago

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

    diff --git a/trac/versioncontrol/api.py b/trac/versioncontrol/api.py
    index 816dcc4..391fbf6 100644
    a b class RepositoryManager(Component):  
    693693                    try:
    694694                        old_changeset = repos.sync_changeset(rev)
    695695                    except NoSuchChangeset:
    696                         self.log.debug(
     696                        self.log.warn(
    697697                            "No changeset '%s' found in repository '%s'. "
    698698                            "Skipping subscribers for event %s",
    699699                            rev, reponame, event)
    class RepositoryManager(Component):  
    707707                        repos.sync_changeset(rev)
    708708                        changeset = repos.get_changeset(rev)
    709709                    except NoSuchChangeset:
    710                         self.log.debug(
     710                        self.log.warn(
    711711                            "No changeset '%s' found in repository '%s'. "
    712712                            "Skipping subscribers for event %s",
    713713                            rev, reponame, event)

comment:3 by Ryan J Ollos, 6 years ago

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

in reply to:  2 comment:4 by Ryan J Ollos, 6 years ago

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

comment:5 by Ryan J Ollos, 6 years ago

Release Notes: modified (diff)

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

comment:6 by Jun Omae, 6 years ago

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?

comment:7 by Ryan J Ollos, 6 years ago

Resolution: fixed
Status: assignedclosed

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 , eas e in [13342].

comment:8 by Ryan J Ollos, 4 years ago

Related issue in #12665.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


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