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

Disable local engines #82

Merged
merged 1 commit into from
Oct 22, 2024
Merged

Disable local engines #82

merged 1 commit into from
Oct 22, 2024

Conversation

r1jt
Copy link
Collaborator

@r1jt r1jt commented Oct 15, 2024

This change will add a configuration variable to disable
installation of local engines lvm and zfs during
umbrella chart based installation

@r1jt r1jt force-pushed the disable-local-engines branch from b2a345b to 4ca78c1 Compare October 15, 2024 13:44
@r1jt r1jt force-pushed the disable-local-engines branch from 7454d5f to ab728d8 Compare October 15, 2024 13:50
@r1jt r1jt requested a review from blaisedias October 15, 2024 15:36
@r1jt r1jt force-pushed the disable-local-engines branch from eb1c758 to fba1d32 Compare October 16, 2024 15:04
@r1jt r1jt requested review from rohan2794 and avishnu October 16, 2024 15:05
@blaisedias
Copy link
Collaborator

Shouldn't this feature be more generic? As in an option to disable any one of the engines?

@r1jt
Copy link
Collaborator Author

r1jt commented Oct 18, 2024

Shouldn't this feature be more generic? As in an option to disable any one of the engines?
Yes. I have now removed changes to handle environment variable setting

@r1jt r1jt force-pushed the disable-local-engines branch from 7887180 to 3a71d9d Compare October 18, 2024 09:14
@blaisedias
Copy link
Collaborator

blaisedias commented Oct 21, 2024

Shouldn't this feature be more generic? As in an option to disable any one of the engines?
Yes. I have now removed changes to handle environment variable setting

The name is still DisableLocalEngines , it should be DisableEngines.
Configuration using an environment variable is a useful flexibility - it should be present.
The same means should be used to disable all IO engines including mayastor.

@r1jt
Copy link
Collaborator Author

r1jt commented Oct 22, 2024

Shouldn't this feature be more generic? As in an option to disable any one of the engines?
Yes. I have now removed changes to handle environment variable setting

The name is still DisableLocalEngines , it should be DisableEngines. Configuration using an environment variable is a useful flexibility - it should be present. The same means should be used to disable all IO engines including mayastor.

I have modified the name to "DisableEngines". However environment variable is not present as I removed the logic to handle it in the pipelines code

@r1jt r1jt force-pushed the disable-local-engines branch from 9d29a53 to 1bedd99 Compare October 22, 2024 07:03
This change will add a configuration variable to disable
 installation of local engines lvm and zfs during
 umbrella chart based installation

Signed-off-by: Ranjith M P <[email protected]>
@r1jt r1jt force-pushed the disable-local-engines branch from 1bedd99 to b6d54ab Compare October 22, 2024 10:50
@rohan2794 rohan2794 merged commit df89f12 into develop Oct 22, 2024
2 checks passed
@rohan2794 rohan2794 deleted the disable-local-engines branch October 22, 2024 11:11
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.

4 participants