-
Notifications
You must be signed in to change notification settings - Fork 14
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
check into themis #81
Comments
This is the code I used to add themis. "themis":
setup: (schema) ->
require("themis").validator(schema)
validate: ({validator, schema, document}) ->
validator(document, '0')
error: (result) ->
if result.valid == true
false
else
result.errors |
I used similar code, and encountered similar errors. The errors seem to come from an expectation in I also checked In fact, of 88 tests, only one had an I'm not closing the ticket, but I am going to de-prioritize testing If it doesn't pass the official tests, then the question is by how much. JSCK decided not to implement a few small pieces of the spec, and many JSON Schema validators have made similar decisions. If this So, temporary |
+1 wontfix. |
I had no problem getting themis to pass the official test suite. If you use require("themis").validator(schema)(document, '0') It works without id's just fine. It's the For reference here is the result I get when running themis against the official testsuite: But we can agree that is a pretty weird implementation themis has for this. |
well, go ahead and post a gist. |
Well there is one test failing for themis:
`Expected result: false but validator returned: "Object # has no method '0'"`` This is probably what disqualifies themis from your benchmarks. |
The latest version of themis However I believe the benchmarks require some improvement. I got widely varying results sometimes while running them. I would suggest using To illustrate my point here's a sample of 5 runs of the benchmark with jsck, imjv and themis Run1Benchmarks for Draft 4Schema: 'Event - Valid document'. A simple schema, exercising very few attributes JSCK Themis[minimal] Themis is-my-json-valid Relative speeds: Schema: 'Configuration'. A moderately complex schema with some nesting and value constraints JSCK Themis[minimal] Themis is-my-json-valid Relative speeds: Schema: 'Transaction'. JSCK Themis[minimal] Themis is-my-json-valid Relative speeds: Run 2Benchmarks for Draft 4Schema: 'Event - Valid document'. A simple schema, exercising very few attributes JSCK Themis[minimal] Themis is-my-json-valid Relative speeds: Schema: 'Configuration'. A moderately complex schema with some nesting and value constraints JSCK Themis[minimal] Themis is-my-json-valid Relative speeds: Schema: 'Transaction'. JSCK Themis[minimal] Themis is-my-json-valid Relative speeds: Run 3Benchmarks for Draft 4Schema: 'Event - Valid document'. A simple schema, exercising very few attributes JSCK Themis[minimal] Themis is-my-json-valid Relative speeds: Schema: 'Configuration'. A moderately complex schema with some nesting and value constraints JSCK Themis[minimal] Themis is-my-json-valid Relative speeds: Schema: 'Transaction'. JSCK Themis[minimal] Themis is-my-json-valid Relative speeds: Run 4Benchmarks for Draft 4Schema: 'Event - Valid document'. A simple schema, exercising very few attributes JSCK Themis[minimal] Themis is-my-json-valid Relative speeds: Schema: 'Configuration'. A moderately complex schema with some nesting and value constraints JSCK Themis[minimal] Themis is-my-json-valid Relative speeds: Schema: 'Transaction'. JSCK Themis[minimal] Themis is-my-json-valid Relative speeds: Run 5Benchmarks for Draft 4Schema: 'Event - Valid document'. A simple schema, exercising very few attributes JSCK Themis[minimal] Themis is-my-json-valid Relative speeds: Schema: 'Configuration'. A moderately complex schema with some nesting and value constraints JSCK Themis[minimal] Themis is-my-json-valid Relative speeds: Schema: 'Transaction'. JSCK Themis[minimal] Themis is-my-json-valid Relative speeds: |
we already have an issue to use anyway, congrats. this looks nice and fast. although this result happened for me just now:
obviously this is another good reason to look at making #29 happen! it could just be a fluke. but if it happens consistently and isn't just a fluke, it might be worth looking into, since I'm pretty sure minimal Themis is supposed to be faster than regular Themis. a few more comments on your PR, #82. also, did you read this comment? I do want to be sure Themis actually validates everything. I asked @MUSCULA for a gist, because he said he got Themis running the JSON Schema official tests. We have a test harness which should be pretty easy to set up. |
The bug that was causing themis to not complete in your benchmarks: `Expected result: false but validator returned: "Object # has no method '0'"`` Was fixed. When I instantiate themis with a schema I use a try catch, and fail all the tests with that schema if it throws. So basically like in #82, but with a try-catch. Be sure to npm update, to get the latest version with the fix. |
@MUSCULA the reason for the However besides this, themis is not supposed to be failing any tests. If you find any bug please do let me know and I'll have it fixed right away. |
@gilesbowkett yes I also got quite a few runs when themis minimal was actually slower than themis default. I even got a run where IMJV was 2 times slower than themis minimal lol. However the benchmarks I use for themis itself uses You can find it here |
@atrniv those look nice and thorough! I might dig into that code to see how you're using |
@gilesbowkett I actually borrowed it from the benchmarks by ZSchema 3 by @zaggino. The credit goes to him. The only thing lacking in it is more practical benchmarks. Right now the only useful benchmarks are the basicObject and the AdvancedObject benchmarks. I'll probably be adding in the benchmarks from jsck into it as well, since they help portray a more rounded perspective of how the validators would perform with actual real world schemas. |
@atrniv thanks! I didn't look at your implementation in detail yet, in fact it could be a week before I can dig into this with any real seriousness. but, fwiw, our benchmarks use five different schemas – two for draft 3 and three for draft 4. combine that with better statistics from |
@MUSCULA @atrniv so I'm still seeing errors all over the place. I've tried it a few different ways:
obviously I'm missing something about how to work with Themis. help me out here. (to be clear, this is when running the JSON Schema test suite, and/or validating against a schema with no top-level |
ok, got this working. will now handle #82. may have to create new branch for it, though, commits to |
@MUSCULA tried to do a PR using the validator library https://github.com/playlyfe/themis with our benchmarks, but ran into trouble.
The text was updated successfully, but these errors were encountered: