-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Use new value cache for json.match_schema
#7081
Use new value cache for json.match_schema
#7081
Conversation
topdown/jsonschema_test.go
Outdated
}, | ||
) | ||
if err != nil { | ||
t.Fatalf("Unexpected schema validation errorerror: %s", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
t.Fatalf("Unexpected schema validation errorerror: %s", err) | |
t.Fatalf("Unexpected schema validation error: %s", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this warrants some docs addition -- now, something else is going to live in the cache, too, and that's relevant for reasoning about a desired value of max_entries
.
topdown/jsonschema_test.go
Outdated
@@ -5,6 +5,8 @@ | |||
package topdown | |||
|
|||
import ( | |||
"context" | |||
"github.com/open-policy-agent/opa/topdown/cache" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] import ordering seems off here
7bf54ba
f0f9611
to
7bf54ba
Compare
✅ Deploy Preview for openpolicyagent ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Added docs @srenatus 👍 |
I figured I'd test this out anyway, and this seemed like a good case given that there was an actual issue on this. Testing response times with OPA running as a server, and the first request is ~800 ms while the following ones are ~10 ms. Fixes open-policy-agent#7011 Signed-off-by: Anders Eknert <anders@styra.com>
7bf54ba
to
291a3c2
Compare
I figured I'd test this out anyway, and this seemed like a good case given that there was an actual issue on this.
Testing response times with OPA running as a server, and the first request is ~800 ms while the following ones are ~10 ms.
Fixes #7011
Why the changes in this PR are needed?
What are the changes in this PR?
Notes to assist PR review:
Further comments: