Skip to content

test/vocabulary: ignore unrecognized optional vocabulary #646

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Mar 7, 2023
Merged

test/vocabulary: ignore unrecognized optional vocabulary #646

merged 1 commit into from
Mar 7, 2023

Conversation

santhosh-tekuri
Copy link
Contributor

working fine with implementation https://github.com/santhosh-tekuri/boon

if test looks fine, will add for draft2019-09 also

@santhosh-tekuri santhosh-tekuri requested a review from a team as a code owner February 27, 2023 16:14
@santhosh-tekuri santhosh-tekuri changed the title test/vocabulary: unrecognized optional vocabulary should be ignored test/vocabulary: ignore unrecognized optional vocabulary Feb 27, 2023
@jdesrosiers
Copy link
Member

This looks good. It exposed a bug in my implementation, but it's correct.

@gregsdennis @Julian Should we include this test in draft-next? When we disallow unknown keywords, this test will require code to identify what keywords are in the vocabulary. Even if the vocabulary is optional, it's necessary to know what keywords are in the vocabulary in order to know if they are allowed to be ignored.

@gregsdennis
Copy link
Member

gregsdennis commented Feb 28, 2023 •
edited
Loading

I think that depends on how we begin to view the optionality of vocabularies. Maybe we need a new discussion to determine if they still make sense in an world where unknown keywords are generally forbidden.

That said, I think we add it to draft-next for now since that's the current rule. We can always remove it later.

@jdesrosiers
Copy link
Member

I think we add it to draft-next for now since that's the current rule. We can always remove it later.

That's fine with me.

@Julian
Copy link
Member

Julian commented Feb 28, 2023

Sounds fine to me too!

@santhosh-tekuri
Copy link
Contributor Author

added to draft2020, draft2019. draft-next

working fine with implementation https://github.com/santhosh-tekuri/boon

jdesrosiers
jdesrosiers approved these changes Mar 6, 2023
View reviewed changes
Copy link
Member

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

Works in my implementation.

gregsdennis
gregsdennis approved these changes Mar 7, 2023
View reviewed changes
Copy link
Member

@gregsdennis gregsdennis left a comment

Choose a reason for hiding this comment

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

These all work for me as well.

@gregsdennis gregsdennis merged commit 6e5d45d into json-schema-org:main Mar 7, 2023
@handrews handrews mentioned this pull request Mar 8, 2023
Closed
handrews added a commit to handrews/jschon that referenced this pull request Mar 9, 2023
@handrews
Auto-create metaschemas referenced in "$schema"
58ebfbc 
This change allows the tests added in
PR json-schema-org/JSON-Schema-Test-Suite#646 to pass,
in accordance with §9.3.1 of the spec, "Detecting a Meta-Schema".

* Catalog.get_metaschema() is added, and is analogous in behavior
  to Catalog.get_schema(), relying on Catalog.create_metaschema()
* It is called from the now-cached JSONSchema.metaschema property,
  to keep the parallel with references which are only resolved
  when used
* Catalog.create_metaschema() now returns the created metaschema
  to avoid having to immediately look it up again
* The core_vocabulary parameters have become default_core_vocabulary
  parameters to allow not knowing the core in advance
* The Metaschema constructor now looks in "$vocabulary" for a
  vocabulary URI matching r'^https://json-schema\.org/draft/[^/]*/core'
  and if it finds a unique match, uses that instead of the default
* Lack of a recognizable core vocabulary still results in a
  JSONSchemaError exception if no default is provided
@handrews handrews mentioned this pull request Mar 9, 2023
Closed
handrews added a commit to handrews/jschon that referenced this pull request Mar 10, 2023
@handrews
Auto-create metaschemas referenced in "$schema"
21405dd 
This change allows the tests added in
PR json-schema-org/JSON-Schema-Test-Suite#646 to pass,
in accordance with §9.3.1 of the spec, "Detecting a Meta-Schema".

* Catalog.get_metaschema() is added, and is analogous in behavior
  to Catalog.get_schema(), relying on Catalog.create_metaschema()
* It is called from the now-cached JSONSchema.metaschema property,
  to keep the parallel with references which are only resolved
  when used
* Catalog.create_metaschema() now returns the created metaschema
  to avoid having to immediately look it up again, as does
  Catalog.create_vocabulary()
* The core_vocabulary parameters have become default_core_vocabulary
  parameters to allow not knowing the core in advance
* The Metaschema constructor now looks in "$vocabulary" for a
  vocabulary URI matching r'^https://json-schema\.org/draft/[^/]*/core'
  and if it finds a unique match, uses that instead of the default
* Lack of a recognizable core vocabulary still results in a
  JSONSchemaError exception if no default is provided
@handrews handrews mentioned this pull request Mar 10, 2023
Merged
handrews added a commit to handrews/jschon that referenced this pull request Mar 13, 2023
@handrews
Auto-create metaschemas referenced in "$schema"
1a24e3c 
This change allows the tests added in
PR json-schema-org/JSON-Schema-Test-Suite#646 to pass,
in accordance with §9.3.1 of the spec, "Detecting a Meta-Schema".

* Catalog.get_metaschema() is added, and is analogous in behavior
  to Catalog.get_schema(), relying on Catalog.create_metaschema()
* It is called from the now-cached JSONSchema.metaschema property,
  to keep the parallel with references which are only resolved
  when used
* Catalog.create_metaschema() now returns the created metaschema
  to avoid having to immediately look it up again, as does
  Catalog.create_vocabulary()
* The core_vocabulary parameters have become default_core_vocabulary
  parameters to allow not knowing the core in advance
* The Metaschema constructor now looks in "$vocabulary" for a
  vocabulary URI matching r'^https://json-schema\.org/draft/[^/]*/core'
  and if it finds a unique match, uses that instead of the default
* Lack of a recognizable core vocabulary still results in a
  JSONSchemaError exception if no default is provided
handrews added a commit to handrews/jschon that referenced this pull request Mar 16, 2023
@handrews
Auto-create metaschemas referenced in "$schema"
0b1108b 
This change allows the tests added in
PR json-schema-org/JSON-Schema-Test-Suite#646 to pass,
in accordance with §9.3.1 of the spec, "Detecting a Meta-Schema".

* Catalog.get_metaschema() is added, and is analogous in behavior
  to Catalog.get_schema(), relying on Catalog.create_metaschema()
* It is called from the now-cached JSONSchema.metaschema property,
  to keep the parallel with references which are only resolved
  when used
* Catalog.create_metaschema() now returns the created metaschema
  to avoid having to immediately look it up again, as does
  Catalog.create_vocabulary()
* The core_vocabulary parameters have become default_core_vocabulary
  parameters to allow not knowing the core in advance
* The Metaschema constructor now looks in "$vocabulary" for a
  vocabulary URI matching r'^https://json-schema\.org/draft/[^/]*/core'
  and if it finds a unique match, uses that instead of the default
* Lack of a recognizable core vocabulary still results in a
  JSONSchemaError exception if no default is provided
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdesrosiers jdesrosiers jdesrosiers approved these changes

@gregsdennis gregsdennis gregsdennis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@santhosh-tekuri @jdesrosiers @gregsdennis @Julian