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

Issue 107 #111

Closed
wants to merge 2 commits into from
Closed

Issue 107 #111

wants to merge 2 commits into from

Conversation

funkyfuture
Copy link
Member

this this still WIP, the PR is intended to run travis' tests and discuss changes.
eventually it shall solve #107.

state of progress:

i haven't yet figured out why the tests do not fail when assertSuccess is used. but i think this is an important issue.

i can confirm that referring self.document in cerberus.py#302 makes a difference.

the failure was then caused by the fact, that in _validate_schema the validator copied itself and the copy then changed its schema-property which would then be reflected in the first validator, that would then continue validation with the subschema, causing the failure as the required field wouldn't exist in the higher context.
therefore i changed the code, so a new instance of a validator was used, not a copy. (on that occasion i made sure a child-validator would inherit all __init__-parameters unless overwritten.

now the problem is that in cerberus.py#227 a given context is always given precedence. which will fail if a subschema is to be validated. but omitting the context (cerberus.py#542) will lead to other failues.

@nicolaiarocci i'd appreciate some advice at this point. especially the code in cerberus.py#227 seems rather strange to me. why would there be a context-parameter which always supersedes the document-parameter? the calling code could / should figure what is to be validated.

@nicolaiarocci
Copy link
Member

context was introduced by @joshvillbrandt in #42 which, in turn, was mainly introduced because of nicolaiarocci/eve/issues/295. I don't have time to look into it right now; hopefully I will in a few days. I'm fine with refactoring of course, as long as we don't introduce regressions.

Josh and/or @nckpark might want to chime in.

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.

None yet

2 participants