Re: [pve-devel] [PATCH v1 pve-manager 0/8] Ceph Build Commit in UI

["Max Carrara" <m.carrara@proxmox.com>]

On Wed Jun 5, 2024 at 4:48 PM CEST, Lukas Wagner wrote:
>
>
> On  2024-04-30 17:28, Max Carrara wrote:
> > Ceph Build Commit in UI - Version 1
> > ===================================
> > 
> > This series adds Ceph's build commit to the UI and lets the user know if
> > a service is running an outdated build and therefore ought to be
> > restarted.
> > 
> > The build commit is now displayed next to the version for all Ceph
> > services like so:
> > 
> >   18.2.2 (abcd1234)
> > 
> > Should a service run an outdated build, the new build commit is also
> > displayed:
> > 
> >   18.2.2 (abcd1234 -> 5678fedc)
> > 
> > (Icons are omitted here).
> > 
> > See the individual patches for more in-depth information.
> > 
> > Additionally, some of the code was also cleaned up and refactored a
> > little along the way.
> > 
> > I'm not 100% sure if the design I've opted for here is the best, so it
> > would be great to get some opinions on this. The OSD tree/list view
> > especially can get a little noisy if there are a lot of outdated OSDs
> > running.
> > 
>
> Neat, gave this a quick test and this seems to do what you describe in the cover letter.
>
> Some comments:
>   - The 'outdated OSD' view in the top-level status widget uses a different icon to
>     signal an outdated OSD than the version column in the OSD tree view - maybe it would make sense
>     to use the same icon here?
>   - The 'Detail' view for OSDs could also show the commit hash, right now it only shows e.g. 18.2.2 (reef)
>
>
> With the nits from the review fixed:
>
> Tested-by: Lukas Wagner <l.wagner@proxmox.com>
> Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>

Thanks a lot for the review and giving this a spin! I'll see if I can
send in a v2 soon. If noone else has any objections, I'll attach your
T-b and R-b tags in the new series.



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel