-
-
Notifications
You must be signed in to change notification settings - Fork 243
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
Create method to Validator that accepts data argument. #267
base: master
Are you sure you want to change the base?
Create method to Validator that accepts data argument. #267
Conversation
I wrote this little script to look for any obvious threading issues. https://gist.github.com/glennpratt/bf254a896c9b0f15b812 No errors on 2.1.6 and JRuby. |
To me this looks fine. 👍 |
This needs work, have issues with shared |
OK, pushed a commit that should fix that issue. Need more tests around that apparently. |
…anges during validation.
fb5dcda
to
bf8e3bf
Compare
Note: This PR is sparse to help decide if this is a good direction to move it. If it is, I'll update usage, documentation and tests. So, do people think this is the right direction? (Thanks @RST-J) |
Yes, this is the right direction. Re-initializing a schema everytime you validate a record is extremely slow. The schema should be initialized once, and then used to validate any number of records. |
Yes I agree with the direction completely. I would actually change the signature of the validate method. We already have a 3.x branch for exactly this kind of backwards incompatible change. Once you're happy with it could open a new pull request on the 3.x branch? |
Currently the Validator class requires data be passed into the constructor, though it doesn't do anything important with it then.
In my case, I'm validating in a server and I see no reason to parse the schema over and over for different inputs, especially since it uses two mutexes in the initialize method.
Let me know if I missed something and Validator simply must accept data at construction.
Is
#validate_data(data)
the way to go, or should I change the signature of#validate
to take an optional argument? The former seemed like a clean change with little API change.