python-jsonschema / check-jsonschema

A CLI and set of pre-commit hooks for jsonschema validation with built-in support for GitHub Workflows, Renovate, Azure Pipelines, and more!
https://check-jsonschema.readthedocs.io/en/stable
Other
192 stars 38 forks source link

FailedDownloadError #294

Closed kaihendry closed 11 months ago

kaihendry commented 11 months ago

In order to check files in config/*.yaml I am using:

    - repo: https://github.com/python-jsonschema/check-jsonschema
      rev: 0.23.3
      hooks:
        - id: check-github-workflows
        - id: check-jsonschema
          name: Check test configs
          files: ^config/[^/]+$
          types: [yaml]
          args: ["--schemafile", "https://raw.githubusercontent.com/EVerest/everest-framework/main/schemas/config.yaml"]

But it fails with:

Check test configs.......................................................Failed
- hook id: check-jsonschema
- duration: 0.56s
- exit code: 1

Error: Unexpected Error building schema validator
FailedDownloadError: got responses with status=200, retries exhausted
  in "/Users/hendry/.cache/pre-commit/repoircq8841/py_env-python3.10/lib/python3.10/site-packages/check_jsonschema/checker.py", line 52
  >>> return self._schema_loader.get_validator(

make: *** [run] Error 1

I don't understand why it fails with status 200 success?

sirosen commented 11 months ago

Thanks for reporting! This is a bug/limitation in terms of how YAML is supported. Also, I think the message can and should be clarified in this case. I'm happy to take suggestions on that if you have ideas after reading my explanation.

The downloader component doesn't just do a download -- it also does a validation step. The idea is to catch a malformed download and retry it.

Looking into the details a bit, the validation callback, although pluggable, is always set to JSON parsing. (This is the bug! :bug: ) So what happens is that we try to download the file and parse it as JSON. That fails, and then we retry in a small loop.

The validation behavior can catch and avoid some nasty cases, so I'd like to keep it. But it needs to be aware that content could be YAML in addition to JSON.

Slightly more broadly, validation needs to expand to support all of the formats check-jsonschema supports (TOML, JSON5), and then try parsing the way it does with a local file. Suffix-less files will still get the JSON behavior -- a bias towards JSON being appropriate for JSON Schema tooling -- but anything .yaml or .yml needs to be supported.

kaihendry commented 11 months ago

idk if you want or not to support YAML, else I'll look to another tool like https://github.com/neilpa/yajsv and attempt to write a pre-commit hook myself 🤷

sirosen commented 11 months ago

I'll never discourage looking at other tools! check-jsonschema is here to help, but if it's not getting your job done, something different might be better for you in the short (or long) term. But YAML is supposed to be supported, so I'll be fixing this as soon as I can dedicate some time to work on the issue.

When YAML support was added, it was intentionally kept very narrow, but it's now sensible to expand to include remote schemas and $refs.

kaihendry commented 11 months ago

Ok, be cool if you figure it out. Thank you Stephen!