oxidecomputer / typify

JSON Schema -> Rust type converter
Apache License 2.0
367 stars 53 forks source link

cargo-typify fails to convert the GitLab CI JSON Schema (draft-07) #611

Closed hawkrives closed 1 month ago

hawkrives commented 1 month ago

Hi! I'm trying to use cargo-typify to generate types for the GitLab CI config file, which they helpfully publish.

$ curl --fail --silent https://gitlab.com/gitlab-org/gitlab/-/raw/master/app/assets/javascripts/editor/schema/ci.json -o gitlab_ci.schema.json

But, when I try to generate the Rust structs, I get a crash.

$ cargo typify gitlab_ci.schema.json --output gitlab_ci.rs
Error: 
   0: Failed to convert JSON Schema to Rust code
   1: Schema conversion failed
   2: value does not conform to the given schema

Location:
   /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cargo-typify-0.1.0/src/lib.rs:167

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.
$ RUST_BACKTRACE=full cargo typify gitlab_ci.schema.json --output gitlab_ci.rs
Error: 
   0: Failed to convert JSON Schema to Rust code
   1: Schema conversion failed
   2: value does not conform to the given schema

Location:
   /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cargo-typify-0.1.0/src/lib.rs:167

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
                                ⋮ 3 frames hidden ⋮                               
   4: cargo_typify::main::hf7ec281ee46f4755
      at <unknown source file>:<unknown line>
   5: std::sys_common::backtrace::__rust_begin_short_backtrace::h9b153ebd38ed268e
      at <unknown source file>:<unknown line>
   6: _main<unknown>
      at <unknown source file>:<unknown line>

Run with COLORBT_SHOW_HIDDEN=1 environment variable to disable frame filtering.

I tried to bisect where the problem definition was, but I got stuck in the middle of testing jobTemplate, which is one of their hairiest schemas in the file. I may try to clone this repo and integrate https://docs.rs/serde_path_to_error/latest/serde_path_to_error/ to see if that gives any more pointers (I'm not sure if it will), but that's dependent on my availability this afternoon.

hawkrives commented 1 month ago

I found the issue in GitLab's schema, in $.defintions.parallel:

"parallel": {
      "description": "Splits up a single job into multiple that run in parallel. Provides `CI_NODE_INDEX` and `CI_NODE_TOTAL` environment variables to the jobs.",
      "oneOf": [
        {
          "type": "integer",
          "description": "Creates N instances of the job that run in parallel.",
          "default": 0,
          "minimum": 1,
          "maximum": 200
        },

They've set the minimum to 1, but the default to 0.

hawkrives commented 1 month ago

I'll go ahead and close this issue – I think there are a number of in-progress tickets that have scoped out better error messages and better error handling, so I don't think that this ticket is required.

Thank you for Typify!

ahl commented 1 month ago

They've set the minimum to 1, but the default to 0.

This is always a quandary: should we do our best to infer something reasonable? Or is the best thing to do to fail.. albeit with more grace than we currently do!

My inclination in this case (and in most cases) is to fail. Ideally we would provide guidance about the specific problem (i.e. "default value is not valid") and a suggestion for how to address it (e.g. a json patch to fix the default).