Skip to content

Conversation

@wedamija
Copy link
Member

@wedamija wedamija commented Dec 21, 2024

This allows us to reference external schemas to avoid duplication. They can be referenced like

"$ref": "file://uptime-results.v1.schema.json#/definitions/CheckResult"

The handler assumes all schemas will be located in out schemas directory.

Usage example here: #361

This allows us to reference external schemas to avoid duplication. They can be referenced like
```
"$ref": "file://uptime-results.v1.schema.json#/definitions/CheckResult"
```

The handler assumes all schemas will be located in out `schemas` directory.
@wedamija wedamija requested review from a team as code owners December 21, 2024 01:04
@wedamija
Copy link
Member Author

I think that we likely need to do something to support this in rust as well - the tests seem less comprehensive there, so although they passed I doubt this works. Any thoughts on this @untitaker?

wedamija added a commit that referenced this pull request Dec 21, 2024
I wanted to avoid duplication in #359, so I wrote #360 to allow us to reference other schemas.
Copy link
Member

@untitaker untitaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(for both rust/python) how does get_schema behave? I think it will have to resolve all file refs and return a self-contained schema. it is used in consumers to validate.

@wedamija
Copy link
Member Author

wedamija commented Dec 21, 2024

(for both rust/python) how does get_schema behave? I think it will have to resolve all file refs and return a self-contained schema. it is used in consumers to validate.

Python works after my changes - #361 has an example. I manually tested dropping fields from the examples and it validates successfully (ie test_examples fails if the schema is invalid). I'll try get a test going in rust

@untitaker
Copy link
Member

@wedamija my bad, yeah i think if we can get some tests in rust that load all schemas like we have in python we'd be golden.

@wedamija wedamija force-pushed the danf/ref-other-schemas branch from 63bee63 to 42f1a81 Compare December 21, 2024 02:49
@wedamija wedamija force-pushed the danf/ref-other-schemas branch from aba7341 to ba6598f Compare December 21, 2024 02:51
wedamija added a commit that referenced this pull request Dec 21, 2024
I wanted to avoid duplication in #359, so I wrote #360 to allow us to reference other schemas.
@wedamija wedamija force-pushed the danf/ref-other-schemas branch from a9a128c to ba6598f Compare December 21, 2024 02:53
@wedamija
Copy link
Member Author

@wedamija my bad, yeah i think if we can get some tests in rust that load all schemas like we have in python we'd be golden.

Ok, this is tested via the schema in #361. I can collapse these prs together if you prefer

@wedamija
Copy link
Member Author

@wedamija my bad, yeah i think if we can get some tests in rust that load all schemas like we have in python we'd be golden.

Ok, this is tested via the schema in #361. I can collapse these prs together if you prefer

Hmm, looks like I missed something in the schema generation...

@wedamija wedamija force-pushed the danf/ref-other-schemas branch from 8b717c9 to be3510d Compare December 23, 2024 21:58
@wedamija wedamija force-pushed the danf/ref-other-schemas branch from 9268c1f to b8f5f77 Compare December 23, 2024 22:07
wedamija added a commit that referenced this pull request Dec 23, 2024
I wanted to avoid duplication in #359, so I wrote #360 to allow us to reference other schemas.
@wedamija wedamija merged commit 52cf555 into main Dec 23, 2024
14 of 16 checks passed
@wedamija wedamija deleted the danf/ref-other-schemas branch December 23, 2024 22:17
wedamija added a commit that referenced this pull request Dec 23, 2024
I wanted to avoid duplication in #359, so I wrote #360 to allow us to reference other schemas.
wedamija added a commit that referenced this pull request Dec 23, 2024
I wanted to avoid duplication in #359, so I wrote #360 to allow us to reference other schemas.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants