Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chopping Block ✂️ 🔥 #21377

Open
10 of 29 tasks
Fryguy opened this issue Aug 13, 2021 · 13 comments
Open
10 of 29 tasks

Chopping Block ✂️ 🔥 #21377

Fryguy opened this issue Aug 13, 2021 · 13 comments

Comments

@Fryguy
Copy link
Member

Fryguy commented Aug 13, 2021

This issue is a list of candidates for removal or serious refactoring due to limited demand of the feature crossed with level of effort in maintenance / footprint

Complete removal:

Possible archiving:

Refactoring / Fixes:

  • oVirt smartstate
    • Does not work in pods, and will likely break in root-less appliances.
    • Better to refactor as having a separate appliance that a user runs in their ovirt system that has access to the disk data, with a VDDK-like API.
  • PXE
    • Might not work in podified due to mount of NFS server. Possibly switch to a FUSE mount.

Needs more discussion

  • Service UI
    • Can we add the missing features to the classic UI? (shopping cart)
  • Policy actions
    • limit the action to just notifications and running automate/playbook
  • External Logging on EmsContainer
    • Remove entirely or link out to the external kibana?
  • "Menu" widgets including the built-in links and quick links
  • custom branding

Other things discussed for removal and NAK'd

  • SCVMM provider
@Fryguy
Copy link
Member Author

Fryguy commented Aug 13, 2021

@Fryguy
Copy link
Member Author

Fryguy commented Aug 13, 2021

I'll open up individual issue for the ones we agreed on

@Fryguy
Copy link
Member Author

Fryguy commented Mar 4, 2022

I added FileDepot to the OP. I believe the only thing it is still used for is log collection, but I'm wondering if we can do log collections completely differently and then just completely delete this class. Let's discuss.

@Fryguy
Copy link
Member Author

Fryguy commented Mar 7, 2022

Added discussion for the removal of "Menu" widgets. These seem completely pointless to me, but maybe I'm misunderstanding some fundamental thing. cc @GilbertCherrie

@Fryguy Fryguy removed this from the Najdorf milestone Mar 7, 2022
@kbrock
Copy link
Member

kbrock commented Mar 10, 2022

can chop custom.css -- Since the file does not have a timestamp, we have a short timeout, and the client checks if the file has changed on every page load.

Also, in the pod world, I do not think that the customer is even able to modify the custom.css.

But this is a business feature and it may be used.

@kavyanekkalapu
Copy link
Member

@Fryguy Removed live metrics in ManageIQ/manageiq-ui-classic#8093

@MelsHyrule
Copy link
Member

Log Collection code deletion is being handled here ManageIQ/manageiq-ui-classic#8235

@jrafanie
Copy link
Member

I added FileDepot to the OP. I believe the only thing it is still used for is log collection, but I'm wondering if we can do log collections completely differently and then just completely delete this class. Let's discuss.

I think we still have code that depends on the FileDepot via the FileDepotMixin.


/Users/joerafaniello/.gem/ruby/2.7.5/bundler/gems/manageiq-ui-classic-bda2273e49af/app/controllers/pxe_controller/pxe_servers.rb
181:      PxeServer.verify_depot_settings(params[:pxe])

/Users/joerafaniello/.gem/ruby/2.7.5/bundler/gems/manageiq-ui-classic-bda2273e49af/app/controllers/application_controller.rb
462:        PxeServer.verify_depot_settings(settings)
joerafaniello@Joes-MacBook-Pro-2 manageiq % git grep FileDepotMixin
app/models/mixins/file_depot_mixin.rb:module FileDepotMixin
app/models/pxe_server.rb:  include FileDepotMixin

I think we can delete the LogFile first and then tackle removing the FileDepotMixin. I'm not familiar with the PxeServer code to know if that depot code can be removed at this time.

@jrafanie
Copy link
Member

Note, I would love to just delete the FileDepot but I think the PxeServer code is still using parts of the FileDepot via the following calls to with_depot:

sync_pxe_images
sync_windows_images
read_file
write_file
delete_file
delete_directory
create_provisioning_files
delete_provisioning_files

@jrafanie
Copy link
Member

jrafanie commented Apr 29, 2022

Note, as I look at this more, FileDepot and subclasses seem to be unrelated to the FileDepotMixin module other than the name being similar.

FileDepotMixin is using 'mount/miq_generic_mount_session' to mount a NFS/SMB/FTP/etc. session and then using that to upload/download/delete files. This is what PxeServer is using. The LogFile class is using FileDepot and subclasses with explicit rows in the database.

FileDepot and subclasses seem to be used for LogFiles, schedules, and MiqServers. We might be able to detangle that and remove the FileDepot and subclasses if PxeServer and FileDepotMixin don't require FileDepot.

But for now, I think LogFile is completely safe to remove.

@Fryguy
Copy link
Member Author

Fryguy commented Feb 16, 2023

@jrafanie @kbrock @agrare @MelsHyrule As discussed, I added #22365 to the Chopping Block

@kbrock
Copy link
Member

kbrock commented Aug 1, 2023

per stale, I added #22642 (based upon a NickL PR that is simple but requires migrations that possibly hit a number of tables)

@Fryguy
Copy link
Member Author

Fryguy commented Nov 8, 2023

Some old Transform VM cleanup: #21874

@Fryguy Fryguy added this to Roadmap Jun 12, 2024
@Fryguy Fryguy moved this to To do in Roadmap Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: To do
Development

No branches or pull requests

5 participants