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

Allow callables for 'default' in schema #815

Closed
wants to merge 1 commit into from

Conversation

dkellner
Copy link
Contributor

@dkellner dkellner commented Feb 1, 2016

Allows the use of calculated default values via callables, e.g. lambda functions. They can even depend on one another and use all other values of the document, while circular dependencies are properly detected: a RuntimeError is raised in this case.

A small example how to use this:

def one_hour_after_value_date(document):
    from datetime import timedelta
    return document['value_date'] + timedelta(hours=1)

RESOURCE_METHODS = ['GET', 'POST']

DOMAIN = {
    'transactions': {
        'schema': {
            'amount': {
                'type': 'integer',
                'required': True
            },
            'expiration_date': {
                'type': 'datetime',
                'default': one_hour_after_value_date
            },
            'value_date': {
                'type': 'datetime',
                'default': lambda doc: doc['_created']
            },
        }
    }
}

@nicolaiarocci
Copy link
Member

This is a great contribution. However, Cerberus 1.0 will bring new normalisation rules, including a brand new default rule. My current plan would be to replace Eve's own default implementation with Cerberus' built-in. This of course can only happen once Cerberus 1.0 is released. I'm hoping to support new Cerberus with Eve 0.7.

If I recall correctly, right now Cerberus's default rule does not support callables, so it would be more appropriate to add support for them in there. There are some more issues however. Validation (and normalisation with v1.0) happens before document meta fields are initialised, so your code snippet would still not work..

Thoughts?

@dkellner
Copy link
Contributor Author

dkellner commented Feb 3, 2016

Hm, too bad :). I really like this feature, in fact, we're using this in production right now. But I think you're right in moving this logic to Cerberus. I will have a look into it in the next days. Maybe I can add callable support based on my changes here to Cerberus.

Do you mean the snippet below by initialization of document meta fields? If yes, we could try to set the future callable-supporting Cerberus-default-rule to a current_utc_datetime function and let Cerberus do the assignment. What do you think?

# Populate meta and default fields
document[config.LAST_UPDATED] = \
    document[config.DATE_CREATED] = date_utc

(eve/methods/post.py, L197)
https://github.com/nicolaiarocci/eve/blob/2d9a69a8e32c566ad9e7a97d99686f7ab84b979c/eve/methods/post.py#L197

@dkellner
Copy link
Contributor Author

dkellner commented Feb 3, 2016

I did a similar implementation for Cerberus: pyeve/cerberus#200.

If you agree, I would change this one to behave exactly the same as the Cerberus one. I.e. passing only the current (sub)document to the callable. This way we could manage backward compatibility if you decide to merge this before Cerberus 1.0 is released.

@nicolaiarocci
Copy link
Member

I'd rather hold onto this one until we merge new Cerberus. Not being a simple fix but, in fact, a new feature, this would go into 0.7, and 0.7 will have to support New Cerberus.

Actually, would you be willing work Cerberus 0.10 (soon to be renamed 1.0) support? I could really use some help there.

@dkellner
Copy link
Contributor Author

dkellner commented Feb 5, 2016

OK, then let's close this and focus on Cerberus 0.10/1.0 and Eve 0.7. I'm going to work on the Cerberus part first and maybe look into Eve integration after that. Unfortunately, I can't guarantee to make it as I'm going to travel for some (longer :)) time - if I start working on it, I'll comment in #776.

@dkellner dkellner closed this Feb 5, 2016
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.

2 participants