feat(connector): resolve $ref in json schema
#20446
Conversation
…hao/use-jsonschema
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
license-eye has checked 5568 files.
| Valid | Invalid | Ignored | Fixed |
|---|---|---|---|
| 2348 | 1 | 3219 | 0 |
Click to see the invalid file list
- src/connector/codec/tests/integration_tests/json.rs
Use this command to fix any missing license headers
```bash
docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix
</details>
| let mut schema = match self.schema_cache.get(&ref_no_fragment) { | ||
| Some(cached_schema) => cached_schema.clone(), | ||
| None => { | ||
| if url_schema == "http" || url_schema == "https" { |
There was a problem hiding this comment.
Are you sure we want to support schema reference from http/fs?
Should we support retrieve references from schema registry instead? https://docs.confluent.io/platform/current/schema-registry/fundamentals/serdes-develop/serdes-json.html#schema-references-in-json-schemas
There was a problem hiding this comment.
Is this feature a customer request? Please provide more context, thanks.
There was a problem hiding this comment.
User only requested schema ref resolution without an external link.
I thought the HTTP support can be a general approve to support schema registry. But I just realized there is a version segment after the subject. I'll change this pr to only support schema resolution without external resource for now.
There was a problem hiding this comment.
Actually, we do allow users using http and file. So it won't hurt to add those support.
It's just for the schema registry, we need to investigate more into how it works.
There was a problem hiding this comment.
Even though schemas are identified by URIs, those identifiers are not necessarily network-addressable. They are just identifiers. Generally, implementations don't make HTTP requests (
https://) or read from the file system (file://) to fetch schemas. Instead, they provide a way to load schemas into an internal schema database. When a schema is referenced by it's URI identifier, the schema is retrieved from the internal schema database.
https://json-schema.org/understanding-json-schema/structuring
There was a problem hiding this comment.
they provide a way to load schemas into an internal schema database
It's true a for a deref library to focus on derefing instead of fetching (in fact it's referencing do). But if we want do do so we will have to ask users to provide all the URL in the schema ref or parse the schema to get all the $ref recurrently before ref.
| let mut schema = match self.schema_cache.get(&ref_no_fragment) { | ||
| Some(cached_schema) => cached_schema.clone(), | ||
| None => { | ||
| if url_schema == "http" || url_schema == "https" { |
There was a problem hiding this comment.
Even though schemas are identified by URIs, those identifiers are not necessarily network-addressable. They are just identifiers. Generally, implementations don't make HTTP requests (
https://) or read from the file system (file://) to fetch schemas. Instead, they provide a way to load schemas into an internal schema database. When a schema is referenced by it's URI identifier, the schema is retrieved from the internal schema database.
https://json-schema.org/understanding-json-schema/structuring
| #[derive(Debug)] | ||
| pub struct JsonRef { |
There was a problem hiding this comment.
Acknowledge that $id and $anchor are currently ignored and unsupported. To be honest I feel like this shall be supported by the json schema library rather than by a hack in the caller that replaces the schema.
There was a problem hiding this comment.
I did check the jsonschema library. It's more like designed for validation and is vary hard to use it for dereffing the schema file.
There was a problem hiding this comment.
shall we move the impl to a standalone crate? I feel that the kernel should not touch the logic of JsonRef.
There was a problem hiding this comment.
No. We may need to hack it to support confluent schema registry. The confluent schema registry should be handled differently. IIUC they can use reference name to retrieve a reference instead of URI.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
close #20351
This PR only resolve the
$refin a json schema. Other fields like$anchor$idetc was not resolved or checked.This PR does not handle the json schema used by confluent schema registry.
Checklist
Documentation
Release note