Timeline should handle being unable to contact subversion better

[chris@…]

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.

[Christian Boos]

Someone reported a similar issue in #2364.

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

[Matthew Good]

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

[Christian Boos]

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:

[Christian Boos]

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

[Christopher Lenz]

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?

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

(exc_info=True).

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

[Christian Boos]

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?

[Christian Boos]

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?

[Christopher Lenz]

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

[Christian Boos]

Fine, as I just finished implementing it :)

See [2878].

[Christian Boos]

… and [2879].