Skip to content

Conversation

@jsoriano
Copy link
Member

@jsoriano jsoriano commented Aug 28, 2023

Allow to define additional parameters for elastic-package stack up from the command line.

Stack providers can access these settings using profile.Config(), they have to provide their own handling and persistence if required.

If the user wants to persist the setting, they need to write it into the config.yml file in the profile.

For example this could be used by the serverless stack provider (#1374) to select the project type:

elastic-package stack up --provider serverless -U stack.serverless.project_type=observability

Allow to define additional parameters for `elastic-package stack up`
from the command line.
@jsoriano jsoriano requested review from a team and mrodm August 28, 2023 11:39
@jsoriano jsoriano self-assigned this Aug 28, 2023
@jsoriano jsoriano marked this pull request as draft August 28, 2023 11:46
@jsoriano
Copy link
Member Author

Moving back to draft. Wondering if this should be directly stored in the profile configuration, and providers would read it from there, as done in https://github.com/elastic/elastic-package/pull/1230/files#diff-1ddc0b543f2deb096c99ee164380726eb20a9728c1ebebaaa8ae1088a9ec5854R147.

@mrodm
Copy link
Contributor

mrodm commented Aug 28, 2023

Moving back to draft. Wondering if this should be directly stored in the profile configuration, and providers would read it from there, as done in https://github.com/elastic/elastic-package/pull/1230/files#diff-1ddc0b543f2deb096c99ee164380726eb20a9728c1ebebaaa8ae1088a9ec5854R147.

In this other PR, I followed the same approach to get some parameters (e.g. project type) from the profile configuration:
https://github.com/elastic/elastic-package/pull/1374/files#diff-b4efb1ac558d15d3077525cb9be59dce12ee723a4749253e9ca7cc860419f1f8R193-R194

If it is done so, would it mean that a user parameter introduced in the command line is going to be re-used in the following executions? if the user does not overwrite it again

elastic-package stack up --provider serverless -U serverless.project_type=security
elastic-package stack down

# this execution would also keep the same value for the parameter `serverless.project_type` from the previous run (security)
elastic-package stack up --provider serverless

It looks like that would match the way of working of provider parameter. Once set in the CLI, it would be kept (stored) until it is overwritten.

@jsoriano
Copy link
Member Author

The thing is that as it is implemented now:

  • <profile>/config.yml is intended for user configuration, this is read with profile.Config(...) I don't think elastic-package should modify this file, as is intended for users.
  • <profile>/stack/config.json contains configuration about the started stack. This file should be written only by elastic-package stack up, and read by any command that needs to access or do anything with the stack.

So if we add settings with CLI flags, where do we persist them?

  • In <profile>/config.yml we would be modifying a file intended for users. If we do this here we would have to try to keep the format, maybe adding the flag overrides at the end. It would persist between executions of stack up.
  • In <profile>/stack/config.json could make more sense, but the providers would need to take care of reading it, and persisting it when needed. It wouldn't persist between executions of stack up.
  • In some other place.

I am more leaned to the second option, but this may complicate a bit configuration management in stack providers. Maybe we can provide helpers for that.

Another option may be to offer a subcommand to manage profile config, instead of adding a flag, and let it manage the profile config.yml. This would be similar to how git config command manages git configuration, but we would also need to try to keep format.

@jsoriano jsoriano marked this pull request as ready for review August 28, 2023 16:35
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @jsoriano

Copy link
Contributor

@mrodm mrodm left a comment

Choose a reason for hiding this comment

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

👍

@jsoriano jsoriano merged commit 53e3867 into elastic:main Aug 29, 2023
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