-
-
Notifications
You must be signed in to change notification settings - Fork 581
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
Very slow validation after dynamicRef implementation, even with schemas which do not use dynamicRef other than in their metaschema #941
Comments
These are the packages we have installed FWIW:
|
Running our unit tests also hangs. I killed the test runner and got this output, it looks like there is maybe a cycle in here?
|
I'd need some sort of reproducer -- there are plenty of tests for validate, so it's certainly not failing in all cases. What schema and instance are you validating either in your tests or real code? |
This is the file that defines our schema: """The json schema for the config file."""
STRING = {"type": "string"}
BOOL = {"type": "boolean"}
DEPLOY_SCHEMA = {
"type": "object",
"properties": {
"resource_group": STRING,
"region": STRING,
"cluster": STRING,
"openshift": BOOL,
"chart": STRING,
"cloud_secret": STRING,
"namespace": STRING,
"tag_name": STRING,
"image_tag": STRING,
"items": {
"type": "array",
"items": {
"type": "object",
"properties": {
"jumpurl": STRING,
"name": STRING,
"namespace": STRING,
"value_file": STRING,
"secret_file": STRING,
"resource_group": STRING,
"region": STRING,
"cluster": STRING,
"openshift": BOOL,
"pr": BOOL
},
"required": ["name"]
}
}
},
"required": ["resource_group", "region", "cluster", "chart", "image_tag", "items"]
}
DEPLOYMENTS_SCHEMA = {
"type": "object",
"items": DEPLOY_SCHEMA
}
BRANCH_SCHEMA = {
"type": "object",
"properties": {
"name": STRING,
"cloud_secret": STRING,
"ns": STRING
},
"required": ["name"]
}
IMAGE_SCHEMA = {
"type": "object",
"properties": {
"registry": {
"type": ["string", "array"],
"items": STRING
},
"namespace": STRING,
"name": STRING,
"branches": {
"type": "array",
"items": BRANCH_SCHEMA
}
},
"required": ["registry", "namespace", "name", "branches"]
}
HELM_SCHEMA = {
"type": "object",
"properties": {
"repos": {
"type": "array",
"items": {
"type": "object",
"properties": {
"name": STRING,
"url": STRING
},
"required": ["name", "url"]
}
}
},
"required": ["repos"]
}
# Which tools are instalable
TOOLS_SCHEMA = {
"type": "object",
"properties": {
"helm": {"type": ["number", "string"]},
"helm_timeout": {"type": ["string"]}
}
}
SCHEMA = {
"type": "object",
"properties": {
"tools": TOOLS_SCHEMA,
"deployments": DEPLOYMENTS_SCHEMA,
"image": IMAGE_SCHEMA,
"helm": HELM_SCHEMA
},
} This is the yaml file being validated: tools:
helm: 3
deployments:
master:
resource_group: Support Services
region: us-south
cluster: support-services
chart: q-site
image_field: qsite_image_tag
namespace: q-site-staging
items:
- name: q-site-dev
value_file: values/values-master.yaml
production:
resource_group: Support Services
region: us-south
cluster: support-services
chart: q-site
image_field: qsite_image_tag
namespace: q-site-prod
items:
- name: q-site-prod
value_file: values/values-prod.yaml |
Thanks, it'd also be helpful if you minimized that to the smallest hanging example. |
I'm not sure what you mean. We had some code that hung validating that exact yaml file (which is parsed and read in using |
What I mean is it's helpful to me or anyone who can spare time to debug if you provide a minimal working example of the issue rather than one with a lot of extra unnecessary complexity. E.g. the issue almost certainly persists if you remove say, the chart properly in DEPLOY_SCHEMA. I'm asking for the smallest possible example demonstrating the problem. If you don't have time to do so you can certainly leave it as is though and someone else may come along to help. |
I have this smaller example from matrix-org/synapse#12649 : import jsonschema
_OEMBED_PROVIDER_SCHEMA = {
"type": "array",
"items": {
"type": "object",
"properties": {
"endpoints": {
"type": "array",
"items": {
"type": "object",
"properties": {
"url": {"type": "string"},
},
},
},
},
},
}
config = [
{
"endpoints": [
{
"url": "https://publish.twitter.com/oembed",
}
],
}
]
jsonschema.validate(config, _OEMBED_PROVIDER_SCHEMA) It seems to be minimal |
4.5.0 is experiencing an infinite loop. python-jsonschema/jsonschema#941
Bisecting against that test example, it seems to be introduced in #886. |
That's really the only change in the release, so it's definitely there, but will still require some minimizing to figure out what the issue is. I'll see if I have a bit of time in a few hours if someone doesn't see the issue by then. (And thanks all for the info so far) |
The example there seems to complete, it's just (very) slow. Specifically here it completes in ~11s. My guess is the original example does too, just even slower, and that the issue is again some missing caching unfortunately. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Hello all! This, along with many many other The next release of It looks from my testing like indeed the examples from this thread work reasonably there -- i.e. aren't unusably slow! If you still care to, I'd love it if you tried out the beta once it is released, or certainly it'd be hugely helpful to immediately install the branch containing this work (https://github.com/python-jsonschema/jsonschema/tree/referencing) and confirm. You can in the interim find documentation for the change in a preview page here. I'm going to close this given it indeed seems like it is addressed by #1049, but feel free to follow up with any comments. Sorry for the delay in getting to these, but hopefully this new release will bring lots of benefit! |
We just started noticing that some tooling which is using this code hangs in 4.5.0:
That seems to hang and there are no warnings or errors. When we drop back to jsonschema<4.5.0 (so 4.4.0) it works. I'm not sure what might be going on here or how to debug.
The text was updated successfully, but these errors were encountered: