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

frontend: PluginSettings: Refactor local storage and plugin data #2671

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

vyncent-t
Copy link
Contributor

@vyncent-t vyncent-t commented Dec 12, 2024

Fixes Issue #2595

Description

  • This PR shrinks the saved JSON data from the plugins information and saves only the name and isEnabled status in local storage. This ensures plugin settings persist when closing and reopening the app.
  • This PR introduces the usePlugins hook into lib/k8s/api/v2
  • It also handles backward compatibility, allowing previously saved settings to be used in this new format.
  • Additionally, it no longer takes old information from the JSON in local storage. Instead, it retrieves the necessary plugin data from the backend when the main Plugin component is called.

Changes

  1. Introduces usePlugins hook: Hook used to fetch the plugins from the backend and return the plugins with their settings
  2. Reduced Storage Data: Only stores essential plugin info (name and isEnabled) in local storage.
  3. Backward Compatibility: Converts old-format data into the new structure on first run.
  4. Data Source Shift: Eliminates use of outdated local storage JSON data, and instead fetches plugin details from the backend when the Plugin component mounts.

How to Test

  1. Setup:
    • Pull this branch and run the application.
  2. Verify Plugin Settings:
    • Open headlamp in app mode
    • Navigate to the settings page then to plugins settings tab
    • Check local storage item headlampPluginSettings
  3. Check Backward Compatibility:
    • Start with an older version of the app or start from main.
    • Navigate to the settings page then to plugins settings tab
    • Check local storage item headlampPluginSettings
    • Switch to this branch and run the app.
    • Confirm that previously stored plugin settings are recognized and converted correctly.
  4. Backend Data:
    • Verify that after loading, the plugin details (other than name and isEnabled) are fetched from the backend and displayed correctly in the UI.
    • Switch plugins off and on and observe the changes in local storage and in app

Notes

  • If you have preexisting local storage data, this PR will convert it automatically. You can reset the local storage if you return to main and go to the same plugin settings paage.
  • Double-check the logs to ensure no errors appear during the migration from old data to new data.

@vyncent-t vyncent-t self-assigned this Dec 12, 2024
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from 005cabd to d2f8668 Compare December 12, 2024 01:01
@vyncent-t vyncent-t marked this pull request as ready for review December 12, 2024 01:03
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Dec 12, 2024
@vyncent-t
Copy link
Contributor Author

Last push adds backwards comp so that all settings wont be set to off or on

@vyncent-t
Copy link
Contributor Author

I have tried to keep the changes as small as possible to accomplish everything it needs to, I am not using the map object to save information anymore, although to hit these targets I still had to use most of what I had reworked previously

@joaquimrocha @sniok

for this PR the targets it needs to hit were:

  • not have the plugins version/data coming from a client-side stored version of it

The plugin data comes from the backend, reused the same method of reaching it from the previous rework

  • avoid having to store all that data when what we are interested in is checking whether the plugins are enabled on the user's side

The data is now trimmed down to just being the name and isEnabled, it is now being used the same way as the original settings where this local saved item is how new app start ups save plugin settings

  • need to be backwards compatible with the previous settings method

The local storage handling logic checks for the old format and changes it to the new format

@vyncent-t vyncent-t marked this pull request as draft December 12, 2024 01:13
@vyncent-t vyncent-t changed the title frontend: PluginSettings: Refactor local storage and plugin data WIP - frontend: PluginSettings: Refactor local storage and plugin data Dec 12, 2024
@vyncent-t vyncent-t marked this pull request as ready for review December 12, 2024 01:16
@vyncent-t vyncent-t changed the title WIP - frontend: PluginSettings: Refactor local storage and plugin data frontend: PluginSettings: Refactor local storage and plugin data Dec 12, 2024
@vyncent-t vyncent-t marked this pull request as draft December 12, 2024 15:03
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from d2f8668 to 56e912f Compare December 12, 2024 18:37
@vyncent-t vyncent-t changed the title frontend: PluginSettings: Refactor local storage and plugin data WIP - frontend: PluginSettings: Refactor local storage and plugin data Dec 12, 2024
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from 56e912f to c150408 Compare December 19, 2024 16:10
@sniok
Copy link
Contributor

sniok commented Dec 19, 2024

Some things to do to avoid duplicating of logic:

  • refactor fetching logic into a single place that both PluginSettings and fetchAndExecutePlugins use
  • use react-query for fetching to have caching and avoid refetching the same stuff

@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from c150408 to 83f279e Compare December 19, 2024 18:45
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from 83f279e to 98c1b58 Compare January 2, 2025 15:24
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from 98c1b58 to 6cea967 Compare January 23, 2025 16:00
@vyncent-t vyncent-t changed the title WIP - frontend: PluginSettings: Refactor local storage and plugin data frontend: PluginSettings: Refactor local storage and plugin data Jan 23, 2025
@vyncent-t vyncent-t marked this pull request as ready for review January 23, 2025 16:01
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Jan 23, 2025
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch 3 times, most recently from b39d2a4 to 93ed4b0 Compare January 27, 2025 20:12
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from 93ed4b0 to 47dfb30 Compare February 6, 2025 15:13
@vyncent-t
Copy link
Contributor Author

pushing rebase of current main

@vyncent-t
Copy link
Contributor Author

note: that last disabled e2e needs to be redone in a later PR as it is out dated with the changes made to two different branches, this branch and the plugins name change branch, this is in my stack for playwright changes I will be working on later

Copy link
Contributor

@sniok sniok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plugins setting stopped working, check the pod-counter plugin settings

@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from d4019b4 to 5d0b9e7 Compare February 13, 2025 17:24
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Feb 13, 2025
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from 5d0b9e7 to f5db3d3 Compare February 13, 2025 17:25
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XXL This PR changes 1000+ lines, ignoring generated files. labels Feb 13, 2025
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch 9 times, most recently from 121d6ea to f71de47 Compare February 20, 2025 16:30
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch 10 times, most recently from f7991dc to f7d5e24 Compare February 25, 2025 14:55
@illume
Copy link
Collaborator

illume commented Feb 28, 2025

@vyncent-t It seems there’s conflicts now. Can you please fix them?

Also, can you please check if the PR description is up to date? I see a lot of changes since the last PR description edit, so I guess if needs updating.

This PR reduces the size of plugin information saved in local storage,
it also introduces the getPlugins hook for fetching plugin information.

Signed-off-by: Vincent T <vtaylor@microsoft.com>
Signed-off-by: Vincent T <vtaylor@microsoft.com>
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from f7d5e24 to 77e1d63 Compare February 28, 2025 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants