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

Add compatibility functions to support multiple server version #16

Closed
wants to merge 4 commits into from

Conversation

torchiaf
Copy link
Collaborator

@torchiaf torchiaf commented Nov 5, 2024

Summary

Implementing custom components and store functions to support multiple server versions.

Usage:

  • Define a component in list/edit/details/models directory, where the root path is the resource name and the suffix is the server version that we want to support.

    example:

    • default component, supports latest server: pkg/harvester/edit/kubevirt.io.virtualmachine/index.vue
    • v1.3.0 component, support server v1.3: pkg/harvester/edit/kubevirt.io.virtualmachine-v1.3.0/index.vue
  • The server version can be specified with the VUE_APP_SERVER_VERSION env variable:

    VUE_APP_SERVER_VERSION=v1.4.0 RANCHER_ENV=harvester API=192.168.0.131 yarn dev
    
  • If VUE_APP_SERVER_VERSION is null, the UI takes the server version from the settings (prod build).

PR Checklists

  • Do we need to backport this PR change to the Harvester Dashboard?
    • Yes, the relevant PR is at:
  • Are backend engineers aware of UI changes?
    • Yes, the backend owner is:

Related Issue

Test screenshot/video

Extra technical notes summary

@torchiaf torchiaf changed the title temp Add compatibility functions to support multiple server version Nov 5, 2024
Signed-off-by: Francesco Torchia <[email protected]>
Signed-off-by: Francesco Torchia <[email protected]>
@torchiaf torchiaf marked this pull request as ready for review November 5, 2024 21:46
@torchiaf torchiaf requested a review from a110605 November 5, 2024 21:58
Copy link
Collaborator

@a110605 a110605 left a comment

Choose a reason for hiding this comment

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

LGTM.

My only concern is there will be 4+ times number of files under edit/list/models/detail folders.

  • v1.3.0
  • v1.3.1
  • v1.3.2
  • latest

and we will have future release

  • v1.3.3
  • v1.4.1

But rootGetters['type-map/hasCustomEdit'] seems not greatly support extra layer folder like edit/v1.3.2/harvesterhci.io.keypair.vue.

@a110605
Copy link
Collaborator

a110605 commented Nov 6, 2024

Another idea is to modify the latest code base to hide those v1.4.0 features based on hasSchema or check with actions or other fields in CRD to make H. extension compatible with v1.3.x.

I like the env variable VUE_APP_SERVER_VERSION that can clearly specify the harvester deployment version to run this extension. We can keep it as one of feature flag.

@aalves08
Copy link
Contributor

aalves08 commented Nov 6, 2024

My opinion is that this pattern will overwrite Dashboard's ResourceList and ResourceDetail, meaning that when the extension is installed, Dashboard will start consuming this.

It's a pattern that was used a couple of times and never yielded any good results/outcome. We've had to revert every single usage of it through time. If you don't track updates on Dashboard for these files that tackle crucial bugs, then your introducing bugs when your extension is installed. It's difficult to track those bug because people rarely make the association of an extension being installed causing a bug somewhere else on Dashboard.

I advise not using it.

One alternative would be to create your "custom" ResourceDetail / ResourceList, but instead rename them to something different like ResourceDetailHarvester / ResourceListHarvester and hijack those entry points for their usage on the context of your extension.

You will still need to maintain them but at least you won't be overriding Dashboard's code because they are "unique".

You'll need to investigate if it's possible to do that kind of isolation on an extension.

Signed-off-by: Francesco Torchia <[email protected]>
@torchiaf
Copy link
Collaborator Author

torchiaf commented Nov 8, 2024

Thanks for all of your suggestions!

We realized that this approach overtakes the compatibility goal. It introduce high code duplication compared to the benefits.
We can keep this code as an example, but we are going to adopt a simpler approach, by defining some featureFlags to control the versions in the UI components: https://github.com/harvester/harvester-ui-extension/pull/19/files#diff-83d43b4bd3f3f3027c37d9e6fe2e31d877ec31a7cda4d5efa64bdc29e26adc9c

@torchiaf torchiaf closed this Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants