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

Make schema coercion to standard data types json-compatible. #316

Open
industrialsynthfreak opened this issue Jun 19, 2017 · 9 comments
Open

Comments

@industrialsynthfreak
Copy link

Hi.

I often use jsons for validation schemas. This helps to provide more info about the config and makes it more versatile. This usually works great with Cerberus. The problems begin when I try to use coercion to standard data types.

So it would be reasonable, as I think, to add bool or string value support for coercion to be automatically converted into a type provided in the "type" param.

Like:

{"type": "integer", "coerce": true} -> {"type": "integer", "coerce": int}

or

{"type": "integer", "coerce": "int"} -> {"type": "integer", "coerce": int}

My apologies if I missed something and it already can do this.

@funkyfuture
Copy link
Member

you could preprocess your schemas with cerberus, couldn't you?

@funkyfuture
Copy link
Member

would it be diabolic to use the builtin callable type to reference the specified type?

but what doesn't make me fond of the idea is that not all types are unambious in regard to their Python counterparts.

@industrialsynthfreak
Copy link
Author

Well, I've just mapped all needed python types to schema type names, and this works kinda fine.
As I understand, almost in the same way it is used in cerberus for type validation, except that it explicitly stores all mappings inside methods while using method names as keys.

The simpliest way would be to use builtins mappings for python types, I guess, but as you've said, theres inconsistency in type names, so...

@funkyfuture
Copy link
Member

with the enhanced possible constraints that i propose in #374 the generating would be possible.

schema = {'ham': {'type': int}}

for field, rules in schema.items():
    if 'type' in rules:
        schema[field]['coerce'] = schema[field]['type']

@funkyfuture
Copy link
Member

funkyfuture commented May 31, 2018

would an instance's configuration flag force_type also be feasible? that'd then apply to any field that has a distinct type defined.

to sum up, the other proposal is to define the behaviour per field by providing either True or type as constraint to coerce.

both approaches require quiet different implementations.

@funkyfuture
Copy link
Member

there's one thing not to forget: the order in which the various normalization rules are applied matters. with the growing amount of possible alterations there's no way around the capability to configure that order per validator instance (and for some that might not be granular enough).

@cpforbes
Copy link

I wanted this capability as well. Here is my idea:

To TypeDefinition add an optional coerce field
AND
add a new normalization rule type_coerce or auto_coerce (boolean, default false)

If type_coerce is true and the TypeDefinition has a coerce field, the type's coerce functions are executed after the functions in the coerce normalization rule.

@funkyfuture
Copy link
Member

funkyfuture commented Jan 18, 2019

so far, there have been different proposals how to address the coercion of field's values based on the constraint which a type rule defines:

1. Transform a schema w/ an external function

a callable (e.g. a Validator instance) is called to adjust a schema that is then used with the designated validator, so e.g. this:

{'field': {'type': 'string'}}

turns into:

{'field': {'coerce': str, 'type': 'string'}}

2. Set implicit coercion with a special constraint for the coerce rule per field

this schema:

{'field': {'coerce': True, 'type': 'int'}}

would signal the validator that it should derive coerce's constraint from the type during normalization phase. alternatives for that indicator could be the builtin type or 'force' or 'auto'.

the problem with that would be, that e.g. the 'list'type maps to an abstract type that cannot be used with coerce.

3. As the previous but defined by an instance config flag

there could also be a force_type flag on a Validator instance that would result in the same behaviour as the previous proposal for all fields. granular definitions are not possible.

the same problem applies here.

4. Adding a target type to errors.TypeDefinition

that comes from @cpforbes's input, i'm ignoring his porposals how to indicate the desired behavior as i think the previous are clearer. but the proposal is able to solve the problem of 2. and 3.


for reasons regarding maintainability i still do prefer the first proposal.

decentral1se added a commit to decentral1se/molecule that referenced this issue Mar 3, 2019
Unfortunately, Cerberus (our schema validation library) is not already
doing this for us. So, we have a bug raised and here is the fix. We're
probably open to numerous version of this bug but let's see when the
reports come in.

Refs:
  * http://docs.python-cerberus.org/en/stable/normalization-rules.html#value-coercion
  * pyeve/cerberus#316

Signed-off-by: Luke Murphy <[email protected]>
@decentral1se
Copy link

decentral1se commented Mar 3, 2019

I've just received a bug and rolled a fix in https://github.com/ansible/molecule/pull/1798/files#diff-1e67bc3147c69bed463615849b56f3b1R683 which I think is another +1 for motivation to resolve this issue. My two cents: if I do 'type': 'string' then I would hope to get that type coercion by default.

EDIT: My solution changed a little in the end, so perhaps not exactly relevant. I had to avoid to accept values of type bool for the value I was coercing (the str coerce was converting them and that could lead to bad things). I wrote a custom coercer.

decentral1se added a commit to decentral1se/molecule that referenced this issue Mar 3, 2019
Closes ansible#1781.

Unfortunately, Cerberus (our schema validation library) is not already
doing this for us. So, we have a bug raised and here is the fix. We're
probably open to numerous version of this bug but let's see when the
reports come in.

Refs:
  * http://docs.python-cerberus.org/en/stable/normalization-rules.html#value-coercion
  * pyeve/cerberus#316

Signed-off-by: Luke Murphy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants