Bug 719536 - list of checkboxes on Excluded Families tab is too long, includes irrelevant families
Summary: list of checkboxes on Excluded Families tab is too long, includes irrelevant ...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Beaker
Classification: Retired
Component: web UI
Version: 0.6
Hardware: Unspecified
OS: Unspecified
medium
unspecified
Target Milestone: 27.0
Assignee: Renan Rodrigo Barbosa
QA Contact:
URL:
Whiteboard: UX
: 707614 (view as bug list)
Depends On:
Blocks: 545868
TreeView+ depends on / blocked
 
Reported: 2011-07-07 08:53 UTC by Jan Hutař
Modified: 2019-11-07 09:06 UTC (History)
9 users (show)

Fixed In Version:
Doc Type: Enhancement
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-11-07 09:06:48 UTC


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Beaker Project Gerrit 6644 0 'None' 'MERGED' 'Rewrite UI of Excluded Families page' 2020-06-22 08:42:39 UTC
Beaker Project Gerrit 719536 0 None None None 2020-06-22 08:42:39 UTC
Red Hat Bugzilla 743077 0 low CLOSED Blacklist "This release or newer" 2021-02-22 00:41:40 UTC
Red Hat Bugzilla 1410495 0 unspecified CLOSED FEATURE REQUEST: Exclude by product line 2021-02-22 00:41:40 UTC
Red Hat Bugzilla 1525664 1 None None None 2021-01-20 06:05:38 UTC
Red Hat Bugzilla 1575632 0 unspecified CLOSED [Beaker 25.2] excluded families contains "lots of bogus entries" 2021-02-22 00:41:40 UTC

Internal Links: 743077 1410495 1525664 1575632

Description Jan Hutař 2011-07-07 08:53:42 UTC
Description of problem:
When I try to reserve with these filtering options:

  Arch: x86_64
  Distro Family: RedHatEnterpriseLinuxServer4
  Tag: RELEASED

I'm being offered some RHEL5 pre-release. Same for ...Client4.

I have been told there is no way how to hide these old and not used distros which are just adding noise to WebUI.


Version-Release number of selected component (if applicable):
Version - 0.6.14


How reproducible:
always


Steps to Reproduce:
1. See reservation form and use filtering options as specified above


Actual results:
Some RHEL5 pre-release offered


Expected results:
That RHEL5 should not be in the list. Ideally, all distros that aren't available on any lab controller should be hidden, and admins should be able to toggle distro visibility on the scheduler.

Comment 2 Vadim Grinco 2011-07-12 14:28:50 UTC
Please make sure you hide them not only from the distro list, but from reserve workflow page, and other, excluded families for a machine, and other places there might be a reference to a distro or an empty distro family.

Comment 3 Šimon Lukašík 2011-07-12 14:29:59 UTC
Possible dupe of bug 707614

Comment 4 Vadim Grinco 2011-07-12 14:40:56 UTC
Simon, only the reserve workflow part is dupe of 707614. I'd include 707614
into this one, as it's with a bit wider requirements.

Comment 5 Dan Callaghan 2012-05-31 22:24:34 UTC
*** Bug 707614 has been marked as a duplicate of this bug. ***

Comment 6 Dan Callaghan 2012-05-31 22:29:17 UTC
This will be addressed in the upcoming Beaker 0.9 release:

* Reserve workflow will not offer distros which are expired from all labs
* Distro and distro tree search pages will not list distros which are expired from all labs

We have not yet done anything about the excluded families checkboxes on the system page (as per comment 2), and that sounds like a good idea, so I will leave this bug open for now.

Comment 8 Dan Callaghan 2014-07-18 01:52:15 UTC
Updating description to reflect the fact that the Excluded Families tab is the only problematic one left.

Comment 9 Nick Coghlan 2014-07-20 23:45:11 UTC
Added to internal dev backlog, since Andrew pointed out how unreadable our Excluded Families tab currently is due to the cruft that has built up over the years. It isn't just old distros - it's ones where the metadata was buggy (e.g. "0" as the OSMajor), so Beaker imported nonsense.

Comment 10 Dan Callaghan 2015-03-25 03:43:19 UTC
The current UI (huge pile of checkboxes) is not really sustainable, given that the number of OS majors in Beaker is continuing to grow rapidly -- even if we were to exclude the ones that are truly cruft.

The exclusions should just be presented as a list of (arch, OS major) and (arch, OS major, OS minor) tuples, which is how they are actually represented in the db. Adding a new exclusion should just mean selecting the arch, typing the OS major (with typeahead assitance, or perhaps a select box) and optionally typing the OS minor, then clicking Add.

This tab is currently one of the last remaining TG widgets so this will be an opportunity to port it to Backbone and add an API as well.

Comment 12 Roman Joost 2018-07-25 02:45:48 UTC
Just to recap the problems we want to address here:

1) In the UI, the list can be insanely long. Each architecture the system supports has it's own list of os majors.

2) Adjusting the excluded families list for a bunch of system is currently not possible or cumbersome at best. A workaround exists by using CSV export/imports (see Bug 1568751 for example).

3) For older hardware in the pool, there is no way to exclude this hardware from any future osmajor which gets added. In other words, there is no way to turn the excluded list into an include list. No UI/API end point currently exists to tell Beaker, that older hardware is only supported up to os major X  (see Bug 743077 for that).

4) It is not possible to exclude a system from an entire family (e.g. CentOS, Fedora) (see Bug 1410495)

I'm currently not certain if all of these points can be addressed without adjusting the database (#3 looks that way).

Comment 13 Dan Callaghan 2018-07-25 02:50:09 UTC
(In reply to Roman Joost from comment #12)
> I'm currently not certain if all of these points can be addressed without
> adjusting the database (#3 looks that way).

Yes certainly #3 and #4 need changes to the way we represent the exclusions. But I guess since this RFE involves a complete rewrite of the UI, that is the best time to do those too.

Comment 14 Roman Joost 2018-07-25 03:51:13 UTC
From a UI point of view I came to the same ideas what Dan has thought of in comment 10. That should cover 1) as well as 2) since the API will be exposed via our CLI, so batch updates should be possible. I'm not sure how well the UI will scale when it comes to poin 3). I envisioned a little switch between inclusion/exclusion, but it has consequences. So in order to avoid confusion, the UI should probably make it clear, that can either exclude or include, not both. Whether that is a separate tab in order to make this distinction clear or not, I don't know yet.

I think at first, lets change the current excluded families with a new API endpoint and changes to bkr client. Then check how we can address the inclusions.

Comment 15 Roman Joost 2018-07-27 02:51:27 UTC
We had a quick chat about the way to go. Altering the excluded and included families will have to become two separate API end points. The UI can then tie this together with a small toggle.

The excluded families end point then can be:

POST /systems/<fqdn>/excluded-distro-trees/

and the data model would be:

* for a whole major version: {'arch': XXX, 'osmajor': XXX}
* for a specific osminor {'arch': XXX, 'osmajor': XXX, 'osminor': '1'}

Comment 16 Dan Callaghan 2018-07-27 02:54:50 UTC
Instead of "included" call it "exclusive". This matches the precedent we have with tasks (excluded arches / exclusive arches, excluded osmajors / exclusive osmajors) which itself matches the RPM model (ExcludeArch / ExclusiveArch).

"Exclusive OS Majors" is clearer than "Included" (included in what?)

Comment 17 Roman Joost 2018-07-31 04:04:01 UTC
I've spend a bit of time on getting back into the JS side of things and to try out some ideas I have with the UI. Terminology as well as templates are all over the shop currently, but this will change.

I've started implementing the API end points for excluded-distro-trees: GET and POST (including happy path tests). Currently the data is based purely on their names, e.g.:

  {'arch': 'i386', 'osmajor': 'CentOS6'}

both for getting and setting. It seems I can uniquely identify OSVersion, OSMajor and Arch by their names, but I'm suspicious if this will not create problems down the road (e.g. in the UI). Suppose if there is need, I can always add the id's of the database objects, but since I do not have to PATCH end points or manipulate the objects directly I think I should be fine. Will see ...

Comment 18 Roman Joost 2018-08-01 07:00:17 UTC
I've got the listing working and working on the removal of items. If I have it wired up right, I think I should be able to get away with a sync() of the collection. I'm currently missing some UI tests, so I need to write those as well (forgot the TDD here...).

One other thing which popped into my mind today is the initial state for the UI if we want to provide a way to edit exclusive and excluding families. If we have two separate API end points, I would have to query both of them to establish what has been set on the system: exclusive or excluding?

Comment 19 Dan Callaghan 2018-08-01 07:05:50 UTC
(In reply to Roman Joost from comment #18)
> One other thing which popped into my mind today is the initial state for the
> UI if we want to provide a way to edit exclusive and excluding families. If
> we have two separate API end points, I would have to query both of them to
> establish what has been set on the system: exclusive or excluding?

The initial state (including whether it is an exclusive list or an exclusion list) should be populated in the page by the server when it renders the template. It shouldn't need to send an AJAX request to the API to fetch the current state.

If both are empty then really it is an exclusion list with no exclusions.

Comment 20 Roman Joost 2018-08-01 22:58:22 UTC
> The initial state (including whether it is an exclusive list or an exclusion
> list) should be populated in the page by the server when it renders the
> template. It shouldn't need to send an AJAX request to the API to fetch the
> current state.
> 
> If both are empty then really it is an exclusion list with no exclusions.

Hm... I do have that advantage in the UI to populate the initial state with server side rendering, but I won't have that with the Beaker client, unless I've missed something. How would you be able to figure out the initial state there?

I had first thought of a data structure like this:

GET /systems/<fqdn>/distro-trees/

{ type: 'exclusive',
  data: [{<list like above>}] }

That would work for both client and UI. Granted, the naming needs work ...

Comment 21 Dan Callaghan 2018-08-02 01:15:25 UTC
Well in the CLI you are not presenting a list of checkboxes... there is no initial state. The CLI should be modelled based on the operations offered in the API. So the operations are adding or removing from the exclusion list -- not replacing the entire thing.

I see two choices for the CLI... either jam more stuff into system-modify using more switches. Then the arch and OS version have to all be encoded together which is super awkward. Something like:

bkr system-modify --add-excluded-distro=osmajor=RedHatEnterpriseLinux7,arch=x86_64

bkr system-modify --add-excluded-distro=osversion=RedHatEnterpriseLinux7.5,arch=x86_64

bkr system-modify --remove-excluded-distro=osmajor=RedHatEnterpriseLinux7,arch=x86_64

bkr system-modify --add-exclusive-distro=osmajor...

But it's probably cleaner to introduce separate subcommands.

bkr system-exclusion-add --osmajor RedHatEnterpriseLinux7 --arch x86_64

bkr system-exclusion-remove --osmajor RedHatEnterpriseLinux7

bkr system-exclusion-add --exclusive --osmajor RedHatEnterpriseLinux6

Also the --arch should really be optional in all of the above. Most systems only support one arch anyway, and even when there is two arches in 99% of the cases the exclusion applies to all the arches anyway.

Comment 22 Roman Joost 2018-08-08 23:53:21 UTC
I want to quickly recapitulate what has been worked on so far:

I started of with the idea of implementing API endpoints which would manipulate a flat list of dictionaries:

[{'arch': XXX, 'osmajor': XXX}]

I realised, that these items don't translate well into backbone. Models should use a uniquely identifiable attribute, which these items lack off. The choice then is to simply add methods to the system model and use jQuery to persist each change in the UI.

However, each model in the database (ExcludeOSMajor, ExcludeOSVersion) does have a unique id (the database id). So my thinking was, since they are uniquely identifiable within their instances and Backbone does have some kind of support for polymorphism (http://backbonejs.org/#Collection-model) all it needs to transparently display the different instances. That indeed did work well up to the point in which I realised that the polymorphism isn't really supported. Each model, no matter what instantiated it, needs a unique ID to be identifiable in the collection.

I could now hack up the collection and models to compute a different identifier, but that sort of defeats the purpose how I started of with: to use Backbone models, which map directly back to our database identifiers and just display them transparently. One additional downside is now to use more than one request to persist models in the database if the system supports more than one architecture. With more than one architecture to support, it also makes displaying tricky. If we want to stop repetition (displaying the same exclusion for each architecture), we need to encode a way to weed out repeating elements, but handle a click for each architecture the element supports.

All this considered, I realised what we want is not a flat list of items like the one stated above, but rather grouped by architecture like so:

[{'arches': [xx], 'osmajor': XXX}]

That eliminates all the work which needs to happen to weed out "duplicate" visual items, and allows to do the hard work of adding/removing exclusions for each architecture at the server end.

Unfortunately that means I'll have to rework parts of my API again, as well as the tests and the UI.


Note You need to log in before you can comment on or make changes to this bug.