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

[RFC] Stop setting default value inside validate method #981

Open
rochacbruno opened this issue Aug 17, 2023 · 1 comment · May be fixed by #988
Open

[RFC] Stop setting default value inside validate method #981

rochacbruno opened this issue Aug 17, 2023 · 1 comment · May be fixed by #988
Assignees
Labels
enhancement Not a Bug Not a Problem, expected behavior RFC
Milestone

Comments

@rochacbruno
Copy link
Member

rochacbruno commented Aug 17, 2023

There is a problem with the current way of setting default values when using validators.

The method _validate_items should have no side effect on the object being validated, inside that method, teh settings must be read only to perform the validation.

Currently this method calls set twice to set default values and apply cast transformation.

here and here

This is causing problems with the inspect history and is duplicating the calls to set

Proposal

  1. Stop calling set on validate_items
  2. Collect defaults and cast from registered validators

Refactoring

  1. Stop calling set and setdefault inside _validate_items
  2. change this block to value = settings.get(name, empty)
  3. On ValidatorList.register iterate all validators and register its default and cast parameter on settings._validators_defaults: DynaBox (OR store it on the instance of ValidatorList itself
  4. On Settings.get line change the implementation to raise KeyError and then on except lookup the self._validators_defaults else default
  5. On line lookup for cast also on self._validators_defaults if not passed directly to get
  6. On Settings init create an instance of ValidatorsList and then call the register on it, also change ValidatorList init to pass accepted args to register instead of super
@rochacbruno rochacbruno added enhancement Not a Bug Not a Problem, expected behavior RFC labels Aug 17, 2023
@rochacbruno rochacbruno added this to the 4.0.0 milestone Aug 17, 2023
@rochacbruno
Copy link
Member Author

This will fix #974 - So the implementation here must also cover the problem reported there

@rochacbruno rochacbruno modified the milestones: 4.0.0, 3.3.0 Aug 22, 2023
@rochacbruno rochacbruno self-assigned this Aug 23, 2023
rochacbruno added a commit that referenced this issue Aug 23, 2023
Default will be declared on validators
but applied only on get method

Fix #981
@rochacbruno rochacbruno linked a pull request Aug 23, 2023 that will close this issue
4 tasks
@rochacbruno rochacbruno modified the milestones: 3.3.0, 3.2.3, 3.2.4 Sep 7, 2023
@pedro-psb pedro-psb modified the milestones: 3.2.4, 3.2.10 Sep 21, 2023
@rochacbruno rochacbruno modified the milestones: 3.2.10, 3.3.0 Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Not a Bug Not a Problem, expected behavior RFC
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants