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

[Resolves #617] allow for configuration path to be specified via CLI.… #647

Closed
wants to merge 2 commits into from
Closed

Conversation

pwrmiller
Copy link

This allows the --config-dir parameter to be specified for the CLI, and allows the config directory to be changed from the default of config.

PR Checklist

  • [:heavy_check_mark:] Wrote a good commit message & description.
  • [:heavy_check_mark:] Commit message starts with [Resolve #issue-number].
  • [:heavy_check_mark:] Added/Updated unit tests.
  • [N/A] Added/Updated integration tests (if applicable).
  • [:heavy_check_mark:] All unit tests (make test) are passing.
  • [:heavy_check_mark:] Used the same coding conventions as the rest of the project.
  • [:heavy_check_mark:] The new code passes flake8 (make lint) checks.
  • [:heavy_check_mark:] The PR relates to only one subject with a clear title.
    and description in grammatically correct, complete sentences.

Approver/Reviewer Checklist

  • Before merge squash related commits.

… The parameter is --config-dir and the directory is specified relative to the sceptre directory, defaulting to 'config'.
@ngfgrant ngfgrant self-assigned this Feb 27, 2019
@ngfgrant ngfgrant self-requested a review February 27, 2019 08:08
Copy link
Contributor

@ngfgrant ngfgrant left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @pwrmiller

In general looks good, we keep the default directory name of "config" but the user can specify a custom name for their config directory, if they want 👍. This is what we had in mind for this part of the project when we developed v2.

I don't this this resolves #617 as here we are just specifying the name of the config folder within the sceptre project. I've made a request to change the SceptreContext parameter config_path to config_directory as I think this better describes what is being set.

Could you update the docs with an example of how to use this added feature?

@@ -40,7 +43,7 @@ class SceptreContext(object):
:type no_colour: bool
"""

def __init__(self, project_path, command_path,
def __init__(self, project_path, command_path, config_path="config",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should probably call this config_directory rather than config_path, project_path and command_path are slightly different as command_path will be given like path/of/stack-groups, project_path is the abs path to the sceptre project.

Copy link
Author

Choose a reason for hiding this comment

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

Agreed. I've changed the name of this parameter to config_directory as requested.

I've also updated docs for the cli, and added an example under architecture.md of how this might change your project structure. This is particularly useful if you have several templates you want to share among projects (e.g. vpc.yaml) to ensure consistency of deployment.

@@ -48,7 +51,7 @@
@click.pass_context
@catch_exceptions
def cli(
ctx, debug, directory, output, no_colour, var, var_file, ignore_dependencies
ctx, debug, directory, config_directory, output, no_colour, var, var_file, ignore_dependencies
Copy link
Contributor

@ngfgrant ngfgrant Feb 27, 2019

Choose a reason for hiding this comment

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

./sceptre/cli/__init__.py:54:100: E501 line too long (102 > 99 characters)

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

…references to config_path to config_directory
```

To deploy project_1 to dev, you would run
`sceptre --config-dir config/project_1 launch dev`
Copy link
Contributor

Choose a reason for hiding this comment

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

@pwrmiller how do you see this being different to:

sceptre launch project_1/dev or sceptre --ignore-dependencies launch project_1/dev

In the the case above you would need to make sure that there were no implicit or explicit dependencies between any of the project configs otherwise sceptre will load them (unless you use --ignore-dependencies).

I think that the main thing here is that if you needed to call your config directory something other than config you could do so with the --config-dir flag.

Another thought is that if you have the same configs that are in dev/test/prod set up you'd ideally want to have one set of files that are deployed as different stacks, which can be achieved with symbolic links.

Copy link
Author

Choose a reason for hiding this comment

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

This is a little different than sceptre launch project1/dev (in fact, if you had the layout in this example, that command results in an error if there are dependencies, as you raised).

You're right that there shouldn't be implicit or explicit dependencies between any project configs - this would produce completely independent project environments (e.g. dev/ qa/ prod) from the same set of templates but different config.

This is desirable for those who like to stand up isolated environments (e.g. separate VPC per project per environment), but where the configuration may differ slightly (e.g. number of AZs).

We could get around this by passing a vars file and using symlinks, but having separate configurations for each project allows for more flexibility in terms of variable configuration without having to pass in var files / templating our stack group configurations. It also allows arbitrary nesting in the config directory (in this example used to define multiple independent environments for multiple projects).

Symlinks are also sometimes an issue on Windows; and although I don't use that platform, we should be cognizant that other users of sceptre might.

@ngfgrant
Copy link
Contributor

ngfgrant commented Jun 3, 2019

Similar to #672 this issue relates to #152

@ngfgrant
Copy link
Contributor

@pwrmiller I just want to check before we move on with this that --dir option would not achieve the same thing?

@pwrmiller
Copy link
Author

pwrmiller commented Jun 28, 2019

@ngfgrant this is separate - the directory to run from (—dir) and the subdirectory for config should default sensibly (as today), but should be allowed to be independent, which this allows.

@ngfgrant
Copy link
Contributor

Ok yeh thought so worth double checking as its been a while since I've looked at this.

@jonylopez84
Copy link

Hello partners. @ngfgrant @pwrmiller
This PR has been stopped?
I have the same need as this PR.
Could we just parse the configuration of the StackGroup that we are deploying?

My scepter structure:

my_sceptre_infra

config
|____infra
| |____config
| | |____dependencies
| | | |____elk
| | | | |____elasticsearch.yaml
| | | | |____config.yaml
| | | | |____entry-route53.yaml
| | |____apps
| | | |____web
| | | | |____alb.yaml
| | | | |____asg-lc.yaml
| | | | |____config.yaml

templates
|____elasticsearch.py
|____alb.py
|____entry_route53.py
|____asg_lc.py

var
|____elk
| |____dev.yaml
| |____common.yaml
| |____prod.yaml
|____web
| |____qa.yaml
| |____common.yaml
| |____prod.yaml

--debug --var-file "var/elk/common.yaml" --var-file "var/elk/dev.yaml" --var "stack_label=xxx" launch infra/config/dependencies/elk -y

I propose this configuration because I want to reuse my templates in any of my applications that use the same resources.

I do the environtment separation in the definition of variables, since I want my configuration to be exactly the same for any environment, although with different values passed as variables.

When I want to launch "elk", scepter also parses the "web" config and fails because it doesn't receive certain variables.
I would like it to just load the config that is in the elk directory.

the --dir option is not useful for this because it refers to the sceptre project_path (where it has to find templates, var, etc.)

@pwrmiller
Copy link
Author

I suspect that this will not be pulled in at this point, since it was opened about 15 months ago. You're welcome to take ownership of this if you still need this functionality @jonylopez84.

Since this was opened I have transitioned away from using Sceptre and thus will not be proceeding with this PR. I'll leave it open for now in case this functionality needs to be added at a later date. Maintainers/ community/ feel free to close or take ownership as necessary.

@thawkson
Copy link

thawkson commented Feb 6, 2021

Are there any outstanding changes that need to considered outside of dealing with the merge conflicts to get this PR merged?

This pull request was closed.
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