Skip to content

Conversation

chrisdl
Copy link

@chrisdl chrisdl commented Oct 1, 2014

Basically an alternate solution to #30

Erroring out of the loop of fields for lookup_data makes it hard to properly catch errors without overriding the entire prepare method (which is fine, but forces people to look at the source code (which is also fine haha)). The problem with this solution is that it does not fail so people will not know if they misspelled their fields, they will need to realize this based on the field they are looking for being null when it really shouldn't be.

My alternate solution (which I have yet to implement and is still brewing in my mind) is to be able to hook into the for loop in the prepare method so that errors to it could be handled on a per-field basis.

I have very split opinions about this over throwing errors. The problem with throwing errors are that they break the loop, so you would have to override the entire prepare method to properly handle them being thrown. I'm super open to suggestions.
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 15c67d2 on chrisdl:master into f39e82c on toastdriven:master.

@chrisdl
Copy link
Author

chrisdl commented Oct 13, 2014

now that pulls #29 and #30 are implemented this is arguably not needed anymore. Also it seems travis is indicating that some of the tests fail on python 3.4. Should I try to make this code work on 3.4 or...?

@chrisdl chrisdl closed this May 12, 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