Bug 312332

Comment 1 Susan McCourt 2010-05-11 10:18:39 EDT

I'd like to take this one...esp in context of the other exception bugs Matthew opened yesterday.  Definitely a problem.

The idea in general of RepositoryTracker is to plug in different repo management strategies into operations, etc., without those subsystems having to know about things like:
- user edit overrides
- colocation assumptions
- error accumulation.

It also provides us a way to plug in a completely different strategy (like the local caching done in the repo manipulation page).  However, it's really targeted to parts of the UI (like viewer elements) where it can be appropriate to "swallow" errors on the assumption they've been reported when first encountered.

However as Matthew has found in several different bugs, the error strategy is not working.  In this bug, we've completely lost the specific exception reporting.  (The refresh is working but the error is swallowed so the UI can't report).

So I want to step back and better define
- who should be using tracker - are there problems for those clients
- the best approach for repo management UI, which needs a more specific solution.  For example, we could simply call the manager refresh API's directly, but I want to ensure we aren't violating an assumption made elsewhere (since this page is API)
- the javadoc for RepositoryTracker should better describe when the mechanisms should be used...

Comment 2 Susan McCourt 2010-05-11 10:22:37 EDT

> The idea in general of RepositoryTracker is to plug in different repo
> management strategies into operations, etc., without those subsystems having to
> know about things like:
> - user edit overrides
> - colocation assumptions
> - error accumulation.

This is also where a lot of the batch eventing occurs which is why I want to be careful here.

Comment 3 Susan McCourt 2010-05-12 15:02:29 EDT

Created attachment 168234 [details]
patch

This patch uses underlying repository manager protocol to refresh repositories so that errors are reported as they happen.

Since the repository tracker is not being used, the event batching in the UI must be changed so that:
- batching is always started, not just when a repo has to be added
- batching is always ended (not conditionally on add)

We don't report artifact repo errors to the user because they have no visiblity of colocation.  Those will get reported when a download is attempted.

The things I tested/need to be validated for any solution:

When repo is already in the pref page when it opens:
- available repo reports as expected
- failed repo reports failure (once, not once for metadata and then artifact)
- when a repo is showing in the combo:
   - disconnect from net
   - reload, failure is reported
   - combo won't change back in the wizard (still shows content)
   - if you select another repo and go back, you see that the repo is not available
   - if you continue in the wizard without reselecting repo, the resolve fails with missing requirement.  Not immediately intuitive, but technically correct since error has already been reported
- when a repo is showing in the combo:
   - disconnect from net but don't reload
   - resolve from net (works because we don't know we are disconnected)
   - complete the wizard (failure in the collect phase because only now we try to load the artifact repo

The other important area to test is reloading repos that were added after the page was open.
- available repo reports as expected.  In addition, the reload doesn't change the selected repo in the install wizard combo
- failed repo reports as expected
- if user cancels out of pref page, the newly tested repo is not in the combo (ie, it didn't really get added)

There are still some oddities in the code, but these are summarized in bug 312683.

Comment 4 Susan McCourt 2010-05-12 15:08:09 EDT

Pascal can you pls review (perhaps with Matthew looking over your shoulder?)

Comment 5 Matthew Piggott 2010-05-12 16:18:12 EDT

I went through the patch and testing and found two small things, both of which seem to have been there before and after the changes (so perhaps should be in separate bugs?)

- Repositories added through the Available Software Page are shown not to have a name until they are used Install New Software dialog.

- If the error returned from a Reload is anything other than 'not found' the repo is not marked in the tracker so no status text is shown in the pref page when the repo is selected.  Interestingly when the same error occurs on the Install New Software page the repos are marked in the tracker, so there is a case when using Reload removes the status text.

Comment 6 Susan McCourt 2010-05-12 16:51:09 EDT

(In reply to comment #5)
> I went through the patch and testing and found two small things, both of which
> seem to have been there before and after the changes (so perhaps should be in
> separate bugs?)
> 
> - Repositories added through the Available Software Page are shown not to have
> a name until they are used Install New Software dialog.

This is bug 230322.  In a nutshell, the producer name isn't known until the repo is loaded by the manager.  

> 
> - If the error returned from a Reload is anything other than 'not found' the
> repo is not marked in the tracker so no status text is shown in the pref page
> when the repo is selected.  Interestingly when the same error occurs on the
> Install New Software page the repos are marked in the tracker, so there is a
> case when using Reload removes the status text.

We intentionally don't remember errors that are anything other than "not found".  The idea is that if it's a "failed read" or some server-side issue, we want to keep trying.  Only when it's "not there" do we want to keep the user from continually hitting the error.

Near the end of 3.5, Henrik and I discussed whether more statuses should be remembered by the repo manager so that we could look up the last failure, etc.  Since it seemed like this might be something done on the manager-side, the UI hasn't bothered to implement its own scheme.  It might be something to take up for 3.7, we keep discussing this stuff at the end of the release when it's too late to make substantive changes.

Comment 7 Pascal Rapicault 2010-05-12 22:03:30 EDT

Fix released.

Comment 8 Susan McCourt 2010-05-17 15:22:08 EDT

verified on Win7, Build id: I20100513-1500