Add classmethod to generate JSON schemas

[jeliebig]

This PR adds a new classmethod json_schema() to generate JSON schemas for YamlConfigDocument classes.

The main motivation for this is getting schemas for riptide config files, but making it possible for all YamlConfigDocument classes might help other projects as well.

I'm not really confident in my Rust programming abilities, but I hope most of the code is fine.

---

Sadly this relies on my fork of schema for now, as the features required to generate useful JSON schemas have not been added yet.

If keleshev/schema#340 gets merged and a new release is created we could switch back to the original version.

The only reason I'm currently doing it this way, is because it requires the least amount of changes to existing projects using configcrunch right now.
In the future it's probably better to replace schema with a different library, which supports everything configcrunch needs out of the box.

[jeliebig]

@theCapypara should I move everything related to JSON schema generation to a new file?
I want to divide json_schema() into smaller functions and start writing tests, but I'd like to know if you are okay with moving everything into a new file before I start.

[jeliebig]

Moving everything to a new file requires the json_schema_id field to be public for the crate.
As long as that won't be a problem, I'd like to move everything to json_schema.rs.

[theCapypara]

@theCapypara should I move everything related to JSON schema generation to a new file? I want to divide json_schema() into smaller functions and start writing tests, but I'd like to know if you are okay with moving everything into a new file before I start.

Moving everything to a new file requires the json_schema_id field to be public for the crate.
As long as that won't be a problem, I'd like to move everything to json_schema.rs.

Sorry, didn't see your comment earlier. Sounds good to me!

[theCapypara]

Hi! I haven't forgotten this, but I need a bit more time to work through it.

In the meantime some questions:

1. Why is main_schema_id needed? Can't just the name of the Schema be used that is returned? I see you added this to the documents.py test class anyway.
2. On that note: main_schema_id is marked as Optional in the Python stub, but is a required argument in the Rust implementation right now:

>>> Project.json_schema(None)
Traceback (most recent call last):
  File "<python-input-3>", line 1, in <module>
    Project.json_schema(None)
    ~~~~~~~~~~~~~~~~~~~^^^^^^
TypeError: argument 'main_schema_id': 'NoneType' object cannot be cast as 'str'

4. Trying to use this PR with Riptide out-of-the-box fails. Reading the docs you added I don't see why it would. Do you know?

>>> Project.json_schema("project")
Traceback (most recent call last):
  File "<python-input-2>", line 1, in <module>
    Project.json_schema("project")
    ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
TypeError: 'NoneType' object cannot be cast as 'str'

In any case the error message should probably be better.

[jeliebig]

1. Why is main_schema_id needed? Can't just the name of the Schema be used that is returned? I see you added this to the documents.py test class anyway.

At first I only added it because Schema.json_schema() required it, but later I wanted to make it as easy as possible to publish the results of the function without needing to make further adjustments to the generated JSON Schemas afterwards.

So if the function is invoked with the URL that the schema(s) will be published at, the resulting schemas could be used directly and all the references to other documents/schemas already work.

I guess we could just fallback to the name of the schema or the name of the document class if the argument isn't provided, but I hadn't thought about that before.

2. On that note: main_schema_id is marked as Optional in the Python stub, but is a required argument in the Rust implementation right now:

As far as I can see, the stub marks main_schema_id as a required parameter:
https://github.com/jeliebig/configcrunch/blob/160b7fe870c3ec87db81b5498a49a24eef65be83/configcrunch/_main.pyi#L55

However json_schema_id for DocReference is marked as optional:
https://github.com/jeliebig/configcrunch/blob/160b7fe870c3ec87db81b5498a49a24eef65be83/configcrunch/_main.pyi#L134-L136

The idea is that you probably don't want to have a dedicated JSON Schema for every document, so you only need to add the argument for those references which should be turned into their own JSON Schema.

4. Trying to use this PR with Riptide out-of-the-box fails. Reading the docs you added I don't see why it would. Do you know?

Schema requires all references to be initialized with name, during testing I simply added a name everywhere and forgot about it.
So to address that, I added a fallback which uses the name of the referenced doc type if the schema wasn't initialized with name.

[jeliebig]

@theCapypara just wanted to check in. Don't worry about taking more time, just want to know whether this PR is still on your radar.

[theCapypara]

Jep, still have this on my list, need to find some time to dive into it, sorry.

[theCapypara]

Thanks again and again sorry for taking so long.

Here's my thoughts

• it would be necessary for schema to merge your PR, as I don't want to have to pull in a fork
• alternatively we could ditch schema and replace it with something else, but from a short research I haven't really seen anything else I'm happy with (that can also provide JSON schemas)

I still don't really understand why there is so much doubling of schema names and IDs. If every schema has a name, why can't this just be used as the JSON schema ID, same for main_schema_id? This also causes having to map between the two because they don't neccesarily match. We also shouldn't make it necessary to give the schema objects names. I guess it's because JSON Schema is an afterthought in schema and its names have a different semantic than JSON schema IDs? But I don't like we have to add names to schemas now for no reason, just to make the schema library happy.

Please correct me if any of my understanding of this is wrong, but I thought of it for a while and in general I think this as-is unfortunately too complex for me to want to maintain as-is.

I also think adding a fallback for non Schema-schemas is too overkill.

Some options:

• We simplify the scope of this PR:
  • Keep using schema but do not rely on it's names or ids in json_schema or DocReference: json_schema() becomes a method without a parameter and DocReference constructor has it's second parameter remved. Instead all documents that want to be compatible with json_schema have to have a new property json_schema_id and provides their ID.
  • AND Require json_schema() to have the document and all $refed documents return a schema object, remove compatibility code for dicts and list.
• Or if we want first-class JSON schema support we should ditch schema and maybe switch to a schema library that works with jsonschema out of the box. Then we require new versions to have schema return that and can just use the functionality of that library. I would prefer that.

[theCapypara]

nit: Since this is also exposed via the Python API, this can just be pub

[theCapypara]

nit: Since this only applies to the first if-branch this return should be moved there for readability

[theCapypara]

Unnecessary return, please run cargo clippy

[jeliebig]

I still don't really understand why there is so much doubling of schema names and IDs. If every schema has a name, why can't this just be used as the JSON schema ID, same for main_schema_id? This also causes having to map between the two because they don't neccesarily match. We also shouldn't make it necessary to give the schema objects names. I guess it's because JSON Schema is an afterthought in schema and its names have a different semantic than JSON schema IDs? But I don't like we have to add names to schemas now for no reason, just to make the schema library happy.

The main thought behind this decision was that people might want to have a different ids when publishing schemas, but maybe that was overkill. I agree it's unnecessarily complicated, so I'd simplify that by using the names everywhere.

Keep using schema but do not rely on it's names or ids in json_schema or DocReference: json_schema() becomes a method without a parameter and DocReference constructor has it's second parameter remved. Instead all documents that want to be compatible with json_schema have to have a new property json_schema_id and provides their ID.
AND Require json_schema() to have the document and all $refed documents return a schema object, remove compatibility code for dicts and list.

I like this option the best, since that would require the least amount of effort for existing projects using configcrunch.

Or if we want first-class JSON schema support we should ditch schema and maybe switch to a schema library that works with jsonschema out of the box. Then we require new versions to have schema return that and can just use the functionality of that library. I would prefer that.

In terms of maintainability I think this would be best, but that would also require more effort for existing projects and that was the initial reason why I didn't go that route initially and instead tried to make it work with the existing schema library.

• it would be necessary for schema to merge your PR, as I don't want to have to pull in a fork
• alternatively we could ditch schema and replace it with something else, but from a short research I haven't really seen anything else I'm happy with (that can also provide JSON schemas)

Since the maintainers of schema don't seem to be that active, don't really engage with open PRs as far as I can see and also allow slop contributions now, I'd be okay with replacing it with something else. Before I opened this PR I didn't know how you wanted to approach this considering backwards compatibility, but if switching to a different (hopefully slop-free) library is an option even though existing projects would need to migrate that would be nice!

I don't want to add a larger maintenance burden for you, so I'd like to go with whatever you think is the best way to ensure you can still maintain this library without problems!