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

Inconsistent validator behavior between copier {copy,recopy} and copier update for computed values #1779

Open
sisp opened this issue Sep 19, 2024 · 2 comments
Labels
Milestone

Comments

@sisp
Copy link
Member

sisp commented Sep 19, 2024

Describe the problem

Currently, there is a difference in behavior regarding the validator expression for computed values (when: false with a default value) between the copier {copy,recopy} operation and copier update. The validator is ignored for copy/recopy but enforced upon update.

This problem came up when configuring a conditional YAML question via when: <expression> with a default value where the default value evaluated to an empty string (i.e., null/None after YAML-parsing) when when evaluated to false while the validator did not handle the null/None value correctly and raised an exception upon copier update.

The reason for this inconsistency is this: During copier {copy,recopy}, only answers passed via the -d, --data (or --data-file) flags or answers from the answers file are validated, so computed values are not validated. But during copier update, the combined answers from the new template's questionnaire are passed to run_copy() via the data argument including the default value from the when: false question, and thus the default value of a when: false question does get validated.

A solution to not validate computed values even on copier update seems to be as follows – I've tested it locally with an additional test and the test suite is passing as well:

-                data=self.answers.combined,  # type: ignore[arg-type]
+                data={},
+                user_defaults=self.answers.combined,  # type: ignore[arg-type]

But it isn't obvious to me what the right behavior is. Should computed values be validated or not when a validator is present? In any case, the behavior should be consistent.

Template

copier-computed-value-validator.zip

To Reproduce

# Unzip the reproducible example above
unzip copier-computed-value-validator.zip
# Change directory into the extracted root directory `copier-computed-value-validator`
cd copier-computed-value-validator/
# Run `copier copy src/ dst/` with the above template and observe no error
copier copy src/ dst/
# Change directory to the destination directory
cd dst/
# Git-initialize the project and make an initial commit
git init
git add .
git commit -m "initial commit"
# Change directory one level up again
cd ..
# Run `copier update dst/` and observe the error
copier update dst/

Logs

$ copier update dst/
No git tags found in template; using HEAD as ref
Updating to template version 0.0.0.post1.dev0+751d554
No git tags found in template; using HEAD as ref
No git tags found in template; using HEAD as ref
Traceback (most recent call last):
  File "$HOME/.local/bin/copier", line 8, in <module>
    sys.exit(copier_app_run())
             ^^^^^^^^^^^^^^^^
  File "$HOME/.local/pipx/venvs/copier/lib/python3.12/site-packages/plumbum/cli/application.py", line 638, in run
    inst, retcode = subapp.run(argv, exit=False)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "$HOME/.local/pipx/venvs/copier/lib/python3.12/site-packages/plumbum/cli/application.py", line 633, in run
    retcode = inst.main(*tailargs)
              ^^^^^^^^^^^^^^^^^^^^
  File "$HOME/.local/pipx/venvs/copier/lib/python3.12/site-packages/copier/cli.py", line 425, in main
    return _handle_exceptions(inner)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "$HOME/.local/pipx/venvs/copier/lib/python3.12/site-packages/copier/cli.py", line 70, in _handle_exceptions
    method()
  File "$HOME/.local/pipx/venvs/copier/lib/python3.12/site-packages/copier/cli.py", line 415, in inner
    with self._worker(
  File "$HOME/.local/pipx/venvs/copier/lib/python3.12/site-packages/copier/main.py", line 228, in __exit__
    raise value
  File "$HOME/.local/pipx/venvs/copier/lib/python3.12/site-packages/copier/cli.py", line 423, in inner
    worker.run_update()
  File "$HOME/.local/pipx/venvs/copier/lib/python3.12/site-packages/copier/main.py", line 899, in run_update
    self._apply_update()
  File "$HOME/.local/pipx/venvs/copier/lib/python3.12/site-packages/copier/main.py", line 966, in _apply_update
    with replace(
  File "$HOME/.local/pipx/venvs/copier/lib/python3.12/site-packages/copier/main.py", line 228, in __exit__
    raise value
  File "$HOME/.local/pipx/venvs/copier/lib/python3.12/site-packages/copier/main.py", line 974, in _apply_update
    new_worker.run_copy()
  File "$HOME/.local/pipx/venvs/copier/lib/python3.12/site-packages/copier/main.py", line 817, in run_copy
    self._ask()
  File "$HOME/.local/pipx/venvs/copier/lib/python3.12/site-packages/copier/main.py", line 491, in _ask
    raise ValueError(f"Validation error for question '{var_name}': {err_msg}")
ValueError: Validation error for question 'computed_value': Should this validator be rendered or not?

Expected behavior

Either the validation is skipped on copier update or the validation error should also occur on copier {copy,recopy}. I'm not sure what's best. Opinions are most welcome.

Screenshots/screencasts/logs

No response

Operating system

Linux

Operating system distribution and version

Ubuntu 22.04

Copier version

copier 9.3.1

Python version

CPython 3.12

Installation method

pipx+pypi

Additional context

No response

@sisp sisp added bug triage Trying to make sure if this is valid or not labels Sep 19, 2024
@yajo
Copy link
Member

yajo commented Sep 21, 2024

I think we should never pass computed values during the update and, instead, let them be computed again.

@yajo yajo removed the triage Trying to make sure if this is valid or not label Sep 21, 2024
@yajo yajo added this to the Soon milestone Sep 21, 2024
@yajo
Copy link
Member

yajo commented Sep 21, 2024

Also maybe we could consider just never validate a computed value. Validators should go into input values instead. However, if we fix the obvious problem from #1779 (comment), this another subject could simply become irrelevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants