#140 discussion tickets get created for bugs not showing in BBA
Opened 4 months ago by kparal. Modified 3 months ago

A ticket 98 got created for bug 1869980. There's some long dependency which make it related to F33 FE tracker:
tracker.png

However, that bug doesn't show up in BBA. So I assume there's some filtering done in BBA which says which bugs to display and which don't. For discussions, we don't seem to perform this filtering and create tickets for everything that appears in BBA database. We should perform the same filtering, so that only bugs showing in BBA frontend have their discussion tickets created.

@lbrabec could you please have a look at this?


Metadata Update from @lbrabec:
- Issue assigned to lbrabec

4 months ago

On the first run of run_cli.py sync-bugs, the bug is created in db and it is actually set as proposed FE and shows in web interface (tested locally).

This is where the discussion ticket gets created which is correct behavior. At this point, the bug is in db as proposed FE.

On the second run of run_cli.py sync-bugs it is set as rejected FE by cleanup_milestone(). Cleanup is called before upating the milestone (where the bug gets created).

At this point, bug is not shown in web interface, but still would be picked by needs_discussion(). This has to be fixed.

However, if I'm reading the code correctly and I'm not missing something, on the first sync (where the bug gets created) the discussion link would be created because the bug is in db as proposed FE (which is corrected by cleanup on the next sync). This is the reason why ticket 101 was created for bug 1794566 as well, but you don't see it now in BBA.

The easiest solution that comes to my mind is to update milestone first and do the cleanup afterwards (flip those two lines). I ran quick check locally and it seems to be doing the trick. However, I'm not sure if this won't cause problems somewhere deep in BBA code.

@tflink, is there a reason why cleanup is called before updating the milesone?

Metadata Update from @lbrabec:
- Assignee reset

4 months ago

Metadata Update from @lbrabec:
- Issue assigned to lbrabec

4 months ago

There is this short documentation to cleanup (I suppose):

   # for each active milestone, update the tracker to remove any bugs that
   # no longer block tracker and pull in any new bugs

IIUIC this is because dependencies of bugs change in time, and that's why we want to check the dependencies (i.e. if a bug still blocks a tracker, or no longer does) before we request updates, to avoid doing too large queries. However, the logic in cleanup_milestone() doesn't match the logic in update_milestone(), and therefore we end up retrieving a bug and showing it in the dashboard for a short while, until the next refresh (30 minutes later) marks the bug as rejected during cleanup and it disappears from the web ui. Does this sound correct?

We could call:

cleanup_milestone()
update_milestone()
cleanup_milestone()

at the expense of some more queries, but that would still create the bug in our DB. It might be enough to paper over the discussion ticket issue, if we adjust our implementation to not create tickets for already rejected bugs. But that all smells like a big steaming mess.

I'd rather solve it properly and not add these bugs into the database at all (which would immediately solve our discussion tickets problem). Which means the update_milestone() logic needs to be adjusted to match cleanup_milestone() logic and bugs like the one above (or here's another one - bug 1794566) aren't saved into the db at all. Another solution is to adjust cleanup_milestone() into a filter, and filter through it all bugs retrieved in update_milestone() first, and only after that save the remainder into the db.

Am I making sense or am I wrong?

I'd rather solve it properly and not add these bugs into the database at all (which would immediately solve our discussion tickets problem).

Isn't there a reason why they are added into the database? I mean, from my naive perspective you could just use get_deps(trackerid) as in celanup_milestone(), but instead there is this crazy recursive logic, why?

I don't think there's a reason for them to be added and then immediately hidden. I think that's a bug. But I haven't wrote the code or studied it very deeply.

@tflink, do you remember?

On the first run of run_cli.py sync-bugs, the bug is created in db and it is actually set as proposed FE and shows in web interface (tested locally).

That's definitely a bug in the sync code, it should be marked rejected the first time that it's synced and I think the only reason it's being modified is that it's not directly depending on the tracker. Cleanup looks for anything that doesn't directly depend on the tracker and marks those bugs as rejected in the db.

Do we really want to be recursing 3 levels when looking for blockers that we care about? I genuinely don't remember why the code goes that far.

It's been a while since I was in this code but if I'm reading this right, it seems like cleanup_milestone() and letting max_recurse=3 in utils/bz_interface.BlockerBugs.flatten_bug_list() are at odds with each other.

Here's another ticket caused by this bug, it seems:
https://pagure.io/fedora-qa/blocker-review/issue/134

@lbrabec Do you think you have enough information now to try to fix it? Thanks a lot.

Do we really want to be recursing 3 levels when looking for blockers that we care about? I genuinely don't remember why the code goes that far.

I'd expect that we should care only about the direct deps of trackers (which is exactly what is left after cleanup_milestone()).

It's been a while since I was in this code but if I'm reading this right, it seems like cleanup_milestone() and letting max_recurse=3 in utils/bz_interface.BlockerBugs.flatten_bug_list() are at odds with each other.

I just tried max_recurse=0, all tests passed and after running sync-bugs (with fresh DB), the web UI shows the same bugs as production instance. This could be the simplest solution, I'll investigate it more tomorrow.

@adamwill Before we remove the transitive dependency tracking, can you please look at this ticket and think of a reason why we currently track blocker dependencies across several levels, but don't display them in the web UI? Are you aware of any reason why this could be helpful somewhere? Thanks.

Well, we do actually want to handle this case:

  • Bug X blocks BetaBlocker (for e.g.)
  • Bug X depends on Bug Y

in that case, Bug Y is actually supposed to be treated as a blocker too. But yeah, the UI has never actually shown this, I wasn't aware we were tracking it on the back end. Maybe no-one could think of a good way of showing it in the UI?

Also, I suppose even though we would ideally show that case in the web UI somehow, we don't need a discussion ticket for Y, only for X.

Hmm, I wonder if this whole thing is too much work to deal with and we should just have a rule that bugs which block accepted blocker/FE bugs automatically get the same status explicitly?

I honestly don't see much gain in showing bug Y in the BBA web UI, or having separate discussion tickets for it. Usually Y gets discussed during bug X discussion, and we often don't care how X gets fixed (whether by fixing Y or something else). There are also projects which use blocks/depends oppositely (I think it was SELinux where all its bugs depended on a tracker bug instead of blocked it), which would cause additional headaches for us. I would probably stop searching for transitive dependencies (maybe we can then skip the cleanup phase? not sure), which will fix extra discussion tickets creation, and we can review it again if we find a good use case for it. There should be no immediate loss of functionality if we do this, because we don't have it hooked up in the web UI anyway. Makes sense?

The main reason to have them tracked somehow is requests. As long as they're not tracked and we don't make them blockers / FEs directly, updates marked as fixing Y but not X (which is normal; classic example is a bug 'Backgrounds not updated' as a blocker, with several dependent bugs like the review request for the new backgrounds and a bug for KDE to update kde-settings) don't show up in the stable push / compose requests; I (or whoever else is doing the request) have to notice them or know about them, and add them manually.

That's a good argument. Does this currently work - are such builds currently present in the generated requests?

Metadata Update from @kparal:
- Issue tagged with: next

3 months ago

Login to comment on this ticket.

Metadata
Boards 1
Next tasks Status: Ideas
Attachments 1
Attached 4 months ago View Comment