Edgewall Software
Modify

Opened 18 years ago

Closed 18 years ago

#2346 closed defect (fixed)

Timeline should handle being unable to contact subversion better

Reported by: chris@… Owned by: Christian Boos
Priority: normal Milestone: 0.10
Component: timeline Version: 0.9
Severity: minor Keywords:
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

Currently, if the subversion repository is down, it gives you some errors such as: ('No such revision 14002', 160006), and some python errors.

Trac is otherwise still usable in this state. The timeline would still be beneficial for those using the ticket system or wiki to monitor changes when a subversion repo goes down for maintenance.

Having some box saying that trac can't contact the subversion repo, to contact the admin, and oh to click here if you want the full error might be more useful than not showing any changes at all.

Attachments (0)

Change History (12)

comment:1 by Christian Boos, 18 years ago

Milestone: 0.9.1
Owner: changed from Jonas Borgström to Christian Boos
Severity: normalminor

Someone reported a similar issue in #2364.

Sending a TracError should be preferable than silently disabling the event provider, I think.

comment:2 by Matthew Good, 18 years ago

How is the Subversion repository "down"? Normally this would imply that the server can't be reached, but since Trac does not have support yet for remote repositories I'm not sure what is being done to take the repository "down".

comment:3 by Christopher Lenz, 18 years ago

Milestone: 0.9.10.9.2

comment:4 by Christian Boos, 18 years ago

Component: generaltimeline
Status: newassigned

What about this?

Index: trac/versioncontrol/web_ui/changeset.py
===================================================================
--- trac/versioncontrol/web_ui/changeset.py     (revision 2707)
+++ trac/versioncontrol/web_ui/changeset.py     (working copy)
@@ -101,8 +101,14 @@
     # ITimelineEventProvider methods
     def get_timeline_filters(self, req):
-        if req.perm.has_permission('CHANGESET_VIEW'):
-            yield ('changeset', 'Repository checkins')
+        if req.perm.has_permission('CHANGESET_VIEW'):
+            # repository unavailability shouldn't affect other event
providers
+            try:
+                self.env.get_repository()
+                yield ('changeset', 'Repository checkins')
+            except TracError, e:
+                yield ('changeset', '(Repository temporarily
unavailable)',
+                       False)
     def get_timeline_events(self, req, start, stop, filters):
         if 'changeset' in filters:

comment:5 by Christian Boos, 18 years ago

Actually, the above should be:

-yield ('changeset', '(Repository temporarily unavailable)',
+yield ('_changeset', '(Repository temporarily unavailable)',

so that selecting the filter (or having an already selected changeset filter) will not trigger the error. Also, to answer mgood's question: a repository can be made temporarily unavailable simply by moving it somewhere else. This can be useful in order to do some kind of maintenance operation without being disturbed by client operations (like a full dump/load, for example).

comment:6 by Christopher Lenz, 18 years ago

  1. I don't think putting the error message in the filters box is that

great. The user can still check the filter and update… what happens in that case?

  1. The error should definitely be logged, including the stack trace

(exc_info=True).

  1. Can we be sure that the TracError indeed means that the repository

is ”temporarily unavailable”? I don't think so. So, how about logging the error, keeping the filter, and then not displaying the changeset events (obviously). Ideally, we would disable the filter checkbox, but that's not possible with the current ITimelineEventProvider API.

comment:7 by Christian Boos, 18 years ago

Answering to cmlenz:

  1. If the user checks the filter and update, nothing happens (see comment 01/02/06 07:18:00). OTOH, I agree that putting the error message there is clumsy. That's was one of the only idea I came with for giving a feedback to the user, though (the other idea was to create a faked error changeset… I think that'd be even worse :) ). Besides modifying the API in order to be able to report errors, of course.
  2. OK
  3. A TracError occurring when simply doing a self.env.get_repository() obviously means that we can't get the repository… As to know if this is only temporary, well, that's just based on the assumption the admin will be able to notice the issue, and recover access.

Simply logging the error and keeping the filter might make the detection of a problem a little harder/slower: you'd have to know that there ought to be changesets but they are not there, before thinking about checking the log… An explicit error feedback would be better, see 1. Disabling the filter checkbox would be a good solution. Maybe the error could be shown as a tooltip for the disabled filter (like Filter disabled: …). We could use a fourth optional return value for get_timeline_filters. If it's a string, it's taken to be an error message and we disable the filter?

comment:8 by anonymous, 18 years ago

Milestone: 0.9.30.9.4

comment:9 by Christian Boos, 18 years ago

I've got an idea: what about a TracError with the appropriate error message and a link to the Timeline with the same set of filters minus the changeset one?

comment:10 by Christopher Lenz, 18 years ago

Milestone: 0.9.40.10

That sounds better, but let's not push this into 0.9.4.

comment:11 by Christian Boos, 18 years ago

Fine, as I just finished implementing it :)

See [2878].

comment:12 by Christian Boos, 18 years ago

Resolution: fixed
Status: assignedclosed

… and [2879].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christian Boos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Christian Boos 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.