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

[user config] exception with nested structures in default context #1149

Closed
lbrack opened this issue Feb 19, 2019 · 13 comments · Fixed by #1516
Closed

[user config] exception with nested structures in default context #1149

lbrack opened this issue Feb 19, 2019 · 13 comments · Fixed by #1516

Comments

@lbrack
Copy link

lbrack commented Feb 19, 2019

Description:

After reading the section on user config and dictionary variables I was under the impression that defining nested YAML data structures under default_context would work (but it fails with a key error exception as is it attempting to access a key that doesn't exist in DEFAULT_CONFIG when iterating recursively through the dictionary resulting from the user config).

After looking at the code the root cause is pretty obvious (and so is the fix) but I am wondering if this is intended behavior (still it shouldn't fail with an exception).

Just for the sake of clarity here is a minimalist user config that makes cookiecutter fail

default_context:
  project_name: my-project
  will_fail:
    on_this: pyops

What I've run:

To reproduce this, copy the YAML example given above in user_config.yml and run
cookiecutter https://github.com/audreyr/cookiecutter-pypackage.git --config-file=user_config.yml
The actual template is irrelevant since the failure occurs before it is even fetched. Here is the backtrace

Traceback (most recent call last):
  File "/usr/local/var/pyenv/versions/user_cfg_problem_3.6/bin/cookiecutter", line 11, in <module>
    sys.exit(main())
  File "/usr/local/var/pyenv/versions/user_cfg_problem_3.6/lib/python3.6/site-packages/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "/usr/local/var/pyenv/versions/user_cfg_problem_3.6/lib/python3.6/site-packages/click/core.py", line 717, in main
    rv = self.invoke(ctx)
  File "/usr/local/var/pyenv/versions/user_cfg_problem_3.6/lib/python3.6/site-packages/click/core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/local/var/pyenv/versions/user_cfg_problem_3.6/lib/python3.6/site-packages/click/core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File "/usr/local/var/pyenv/versions/user_cfg_problem_3.6/lib/python3.6/site-packages/cookiecutter/cli.py", line 120, in main
    password=os.environ.get('COOKIECUTTER_REPO_PASSWORD')
  File "/usr/local/var/pyenv/versions/user_cfg_problem_3.6/lib/python3.6/site-packages/cookiecutter/main.py", line 54, in cookiecutter
    default_config=default_config,
  File "/usr/local/var/pyenv/versions/user_cfg_problem_3.6/lib/python3.6/site-packages/cookiecutter/config.py", line 109, in get_user_config
    return get_config(config_file)
  File "/usr/local/var/pyenv/versions/user_cfg_problem_3.6/lib/python3.6/site-packages/cookiecutter/config.py", line 76, in get_config
    config_dict = merge_configs(DEFAULT_CONFIG, yaml_dict)
  File "/usr/local/var/pyenv/versions/user_cfg_problem_3.6/lib/python3.6/site-packages/cookiecutter/config.py", line 54, in merge_configs
    new_config[k] = merge_configs(default[k], v)
  File "/usr/local/var/pyenv/versions/user_cfg_problem_3.6/lib/python3.6/site-packages/cookiecutter/config.py", line 54, in merge_configs
    new_config[k] = merge_configs(default[k], v)
KeyError: 'will_fail'

Version

$ cookiecutter --version
Cookiecutter 1.6.0 from /usr/local/var/pyenv/versions/user_cfg_problem_3.6/lib/python3.6/site-packages (Python 3.6)
@lbrack lbrack changed the title [user config] user config exception with nested structures in default context [user config] exception with nested structures in default context Feb 19, 2019
lbrack pushed a commit to lbrack/cookiecutter that referenced this issue Feb 20, 2019
Fixed KeyError exception when YAML user config file contains nested structures
by settting the default's default to an OrderedDict if the key does not exist.

Added test_get_config:test_get_config_with_nested_struct_defaults to cover
that case
@insspb insspb added the 1.8.0 label Dec 22, 2019
@insspb insspb added this to To do in 1.8.0 Release via automation Dec 22, 2019
@bryanburke
Copy link

This bug seems to be due to how cookiecutter.config.merge_configs recursively steps into dictionaries beyond the first level of the config file's nested structure. I played around with the code in that function, and I found two possible solutions:

# Possible solution 1: Use dict.update instead of recursing.
if isinstance(v, dict):
    new_config[k].update(v)

# Possible solution 2: Only recurse for known top-level keys.
if isinstance(v, dict) and k in DEFAULT_CONFIG:
    new_config[k] = merge_configs(default[k], v)

This issue is likely also related to #1232.

@lbrack
Copy link
Author

lbrack commented Jan 16, 2020

It's been a while ... so I had to refresh my memory.

I think solution 1 wouldn't work because if the user config is a subset of the entire config (why repeat config options we want to keep as default) then any nested dictionary would override whatever dictionary is at that level e.g.

default = dict(a=1, b=2, c=dict(c1=3, c2=4))
user = dict(a=1, b=4, c=dict(c1=5))
default.update(user)
# Resulting in {'a': 1, 'b': 4, 'c': {'c1': 5}}

As far as solution 2, the recursion would only happen if k is in DEFAULT_CONFIG, which, unless I am not understanding something, would not solve the problem.

Here is how I had fixed the problem. Not sure if this is the most elegant way to solve it but it does work.

def merge_configs(default, overwrite):
    """Recursively update a dict with the key/value pair of another.

    Dict values that are dictionaries themselves will be updated, whilst
    preserving existing keys.
    """
    new_config = copy.deepcopy(default)

    for k, v in overwrite.items():
        # Make sure to preserve existing items in
        # nested dicts, for example `abbreviations`
        if k not in overwrite:
            raise KeyError("option '{}' is not defined in configuration {}".format(k, overwrite))
        if isinstance(v, dict):
            new_config[k] = merge_configs(default.setdefault(k, collections.OrderedDict([])), v)
        elif isinstance(overwrite[k], list) is True and isinstance(v, list) is False:
            # The default option is a list. If the value is a scalar, we need to make sure
            # that v is in the list
            if v not in overwrite[k]:
                raise ValueError("value '{}' of option '{}' "
                                 "is not in the list of "
                                 "choices {}".format(v, k, overwrite[k]))
            new_config[k] = v
        else:
            new_config[k] = v
    return new_config

@ssbarnea ssbarnea removed the 1.8.0 label Mar 30, 2020
@TheErk
Copy link

TheErk commented Apr 8, 2020

I am hit by the same issue.
I have a top-level entry in cookiecutter.json which is:

"algorithms": {
    "dummy" : {
      "id" : "dummy_algo",
      "roles": ["part1", "part2"]
  }

and I want to be able to replace the content of algorithms with this (in my user-config.yaml):

algorithms:
    algo1:
      id: "myalgo1"
      roles: 
        - part1
        - part2
    algo2:
      id: "myalgo2"
      roles:
        - part1

and the solution proposed by @lbrack fixes my issue.
Any chance this can be integrated in forthcoming release?

@TheErk
Copy link

TheErk commented Apr 8, 2020

@ssbarnea you removed this from 1.8.0 could you elaborate why? Without this user-overriding of dict variables is not possible.
Although you can if you do it by passing the dict var on the command line as 'extra_context':
cookiecutter .... algorithms="{'algo1': {'id': 'myalgo1', 'roles': ['part1', 'part2']}}, 'algo2': {'id': 'myalgo2', 'roles' : ['part1']}}"
which is cumbersome.

in the light of this another solution would be to treat those dict var as:

extra_context:

in the yaml config file.

@ssbarnea
Copy link
Member

ssbarnea commented Apr 8, 2020

I removed the label and assigned the milestone because labels should not be used with version planning for which we already have milestones (or projects).

@TheErk
Copy link

TheErk commented Apr 8, 2020

ok thanks for the workflow explanation @ssbarnea

@alzafacon
Copy link

For those who need a workaround today, you can just overwrite the cookiecutter.json file with the values you want. and run cookiecutter --no-input <path/to/cookiecutter>

@gitttt
Copy link

gitttt commented Apr 22, 2021

I applied the fix from @simobasso and it worked.

@simobasso
Copy link
Member

just merged #1516 🎉
Thanks everybody 🙏 !

@paul-michalik
Copy link

Well but with cookiecutter version 1.7.3 this still does not work...

@glumia
Copy link
Contributor

glumia commented Jun 13, 2021

@paul-michalik changes of #1516 were not yet released.

@paul-michalik
Copy link

@paul-michalik changes of #1516 were not yet released.

When do you plan to release it? Is it scheduled for specific version?

@glumia
Copy link
Contributor

glumia commented Jun 13, 2021

It should be released with 2.0.0, check #1555!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
1.8.0 Release
  
To do
Development

Successfully merging a pull request may close this issue.

10 participants