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

Fix validation of steps #1176

Merged
merged 6 commits into from
Jul 6, 2018

Conversation

jenshnielsen
Copy link
Collaborator

Fixes #1174

Ensure that intermediate steps are validated too. In general we cannot assume that a step is valid just because the endpoint is

@jenshnielsen
Copy link
Collaborator Author

jenshnielsen commented Jul 5, 2018

This failed because the number of validation calls generated have changed. Intermediate steps are validated after setting with the following logic.

self._save_val(val_step,
    validate=(self.val_mapping is None and
                   self.set_parser is None and
                   not(step_index == len(steps)-1 or
                   len(steps) == 1)))

I am not sure I see the need for that if we validate them up front too. Perhaps we should get rid of it. Validating before setting seems like the correct thing to do in any case

@codecov
Copy link

codecov bot commented Jul 5, 2018

Codecov Report

Merging #1176 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1176      +/-   ##
==========================================
- Coverage   79.67%   79.66%   -0.02%     
==========================================
  Files          47       47              
  Lines        6661     6662       +1     
==========================================
  Hits         5307     5307              
- Misses       1354     1355       +1

@astafan8 astafan8 self-requested a review July 5, 2018 15:36
@jenshnielsen jenshnielsen merged commit d2abd73 into microsoft:master Jul 6, 2018
giulioungaretti pushed a commit that referenced this pull request Jul 6, 2018
Merge: 0d85481 f7469d1
Author: Jens Hedegaard Nielsen <[email protected]>

    Merge pull request #1176 from jenshnielsen/fix_validation_of_steps
@jenshnielsen jenshnielsen deleted the fix_validation_of_steps branch November 6, 2018 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants