Skip to content
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.

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

Add language clarifying multiple data schemas #1023

Closed
wants to merge 7 commits into from

Conversation

decentralgabe
Copy link
Contributor

@decentralgabe decentralgabe commented Feb 1, 2023

Addresses #950


💥 Error: 500 Internal Server Error 💥

PR Preview failed to build. (Last tried on Feb 6, 2023, 6:15 PM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 Spec Generator - Spec Generator is the web service used to build specs that rely on ReSpec.

🔗 Related URL


😭  Sorry, there was an error generating the HTML. Please report this issue!
Specification: http://labs.w3.org/spec-generator/uploads/0GVPUy/index.html?isPreview=true%3FisPreview%3Dtrue&publishDate=2023-02-06
ReSpec version: 32.7.0
File a bug: https://github.com/w3c/respec/
Error: Error: Evaluation failed: Timeout: document.respec.ready didn't resolve in 28323ms.
    at ExecutionContext._ExecutionContext_evaluate (/u/spec-generator/node_modules/puppeteer-core/lib/cjs/puppeteer/common/ExecutionContext.js:229:15)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async ExecutionContext.evaluate (/u/spec-generator/node_modules/puppeteer-core/lib/cjs/puppeteer/common/ExecutionContext.js:107:16)
    at async generateHTML (/u/spec-generator/node_modules/respec/tools/respecDocWriter.js:221:12)
    at async toHTML (/u/spec-generator/node_modules/respec/tools/respecDocWriter.js:92:18)
    at async Object.generate [as respec] (file:///u/spec-generator/generators/respec.js:15:44)
    at async file:///u/spec-generator/server.js:252:48

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

Comment on lines +2462 to +2463
The value of the <code>credentialSchema</code> <a>property</a> MUST be a single
data schema object which provides <a>verifiers</a> with enough information
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, this makes it not possible to provide more than one type of schema... for example, one schema to do data validation (e.g., JSON Schema) and another schema to do data transformation (e.g., CL Signatures, Gordian Envelopes, etc.).

Limiting to ONE and only one schema is probably not a good idea.

Limiting to one schema type in the list of credentialSchemas is probably better.

Copy link
Contributor

Choose a reason for hiding this comment

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

Limiting to ONE and only one schema is probably not a good idea.

If we don't limit to one, we must provide examples that make use of more than one, and tests...

I suggest limiting to one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes - suggest limiting to one schema type in the list of credential schema, which when resolved can contain multiple schemas

Copy link

Choose a reason for hiding this comment

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

If we don't limit to one, we must provide examples that make use of more than one, and tests...

I agree with this, we need to weigh up the complexity that supporting multiple schemas introduces against possible usecases, every feature can also be a bug. I also don't think using a schema to do data transformation is the right use of a schema and doesn't really match what this feature is described as being for in the current standard.

index.html Outdated Show resolved Hide resolved
index.html Outdated
data schema object which provides <a>verifiers</a> with enough information
to determine if the provided data conforms to the provided schema object. Each
<code>credentialSchema</code> MUST specify its <code>type</code> (e.g.,
<code>JsonSchemaValidator2018</code>,
Copy link
Member

Choose a reason for hiding this comment

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

We should remove JsonSchemaValidator2018 since it never existed in practice and was just suggested as an example, IIRC.

Copy link
Contributor

Choose a reason for hiding this comment

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

100% agree.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will remove.

index.html Outdated
to determine if the provided data conforms to the provided schema object. Each
<code>credentialSchema</code> MUST specify its <code>type</code> (e.g.,
<code>JsonSchemaValidator2018</code>,
<code><a href="https://w3c-ccg.github.io/vc-json-schemas/">CredentialSchema2022</a></code>,
Copy link
Member

Choose a reason for hiding this comment

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

We should link to this one non-normatively.

Copy link
Contributor

Choose a reason for hiding this comment

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

We need a normative link, or the feature needs to be removed from the specification.

It could be this one, or the one I proposed to the list which the traceability community uses.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How can I link to this non-normatively?

What about keeping it non-normative until v2 is shored up (in progress).

index.html Show resolved Hide resolved
index.html Outdated
Comment on lines 2468 to 2469
or <code><a href="https://transmute-industries.github.io/vc-credential-schema-open-api-specification/">
OpenApiSpecificationValidator2022</a></code>), and an <code>id</code> <a>property</a>
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't link to vendor organization Github repos in standards-track specs because 1) not all vendor organizations stick around for as long as these specs do and the links break over time, and 2) competing vendors complain of low-key favoritism when we do this.

Ideally, all external links should point to stable documents that we expect to last for a decade or more.

Copy link
Contributor

Choose a reason for hiding this comment

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

I proposed this item for the group to consider on the mailing list, perhaps we can pull the work item into the group?

Then the W3C can manage hosting of the document, and the references would be internal.

Copy link
Contributor

Choose a reason for hiding this comment

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

The other reference is to the W3C CCG which also relies on "GitHub" for hosting.

Copy link
Contributor

Choose a reason for hiding this comment

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

Additional context, there are at least 4 companies using this approach today, its the same approach used by

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@OR13 can you move it into the CCG and I'll add it back then?

Copy link
Contributor

Choose a reason for hiding this comment

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

@decentralgabe we are not required to "move things to CCG" by W3C Process.

We can adopt them directly as work items, as a working group.

I suggest adding a note saying it will be removed in future draft if its not adopted as a work item, to both links.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Waiting for you and @msporny to have consensus before proceeding here

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Copy link
Member

@msporny msporny left a comment

Choose a reason for hiding this comment

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

Mostly editorial changes requested. The one item that's concerning is the limitation of only being able to associate a single schema with a VC, which removes composability of the credentialSchema feature.

@OR13
Copy link
Contributor

OR13 commented Feb 1, 2023

Assuming issues are filed for disagreements, this is an improvement and should be accepted.

decentralgabe and others added 3 commits February 1, 2023 08:23
Co-authored-by: Manu Sporny <msporny@digitalbazaar.com>
Co-authored-by: Manu Sporny <msporny@digitalbazaar.com>
index.html Outdated Show resolved Hide resolved
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
index.html Show resolved Hide resolved
@iherman
Copy link
Member

iherman commented Feb 6, 2023

The issue was discussed in a meeting on 2023-02-01

  • no resolutions were taken
View the transcript

3.5. Add language clarifying multiple data schemas (pr vc-data-model#1023)

See github pull request vc-data-model#1023.

Manu Sporny: New one PR 1023 from yesterday, about credential schemas. This PR says you can have only one credential schema, and then there's some clarifying language. Please jump in if you have opinions on credential schema..
… This is all about VC data model..

Manu Sporny: https://github.com/w3c/vc-status-list-2021/pulls.

Manu Sporny: Moving on to vc-status-list-2021 PRs.

Copy link

@matthieubosquet matthieubosquet left a comment

Choose a reason for hiding this comment

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

Delegating composability to an hypothetic future implementation type of schema validation seems ok to me.

I’m not sure it requires an entire paragraph (since cardinality is clear, I think it would be ok to remove that note/the second paragraph altogether).

The cardinality of 1 schema is clear to me.

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
@decentralgabe
Copy link
Contributor Author

I believe the PR is blocked until we resolve whether schemas can be a single (and solve multiple in the type) or multiple (and add language here on how to process multiple). I believe the former approach is simpler.

Shall we aim to discuss this on an upcoming call?
cc: @msporny @David-Chadwick

@David-Chadwick
Copy link
Contributor

yes happy to

@decentralgabe decentralgabe mentioned this pull request Feb 15, 2023
@iherman
Copy link
Member

iherman commented Feb 16, 2023

The issue was discussed in a meeting on 2022-09-15

  • no resolutions were taken
View the transcript

5.4. Verification of multiple credential schemas (issue vc-data-model#950)

See github issue vc-data-model#950.

Gabe Cohen: There is a PR.

See github pull request vc-data-model#1023.

@msporny msporny added DO NOT MERGE PR contains something that should not be merged. discuss labels Feb 22, 2023
@decentralgabe
Copy link
Contributor Author

After today's call I'm ready to close this PR and move back to discussion in the issue #950

Do you think that's reasonable - @dlongley @dmitrizagidulin @OR13 ?

@iherman
Copy link
Member

iherman commented May 3, 2023

The issue was discussed in a meeting on 2023-05-03

  • no resolutions were taken
View the transcript

3.2. Add language clarifying multiple data schemas (pr vc-data-model#1023)

See github pull request vc-data-model#1023.

Brent Zundel: nobody wanted to speak to that.
… next up is PR 1023.
… gabe, what needs to be done to move this forward?

Kristina Yasuda: PR 1035 should be closed when rendering gets added to the reserved table.

Gabe Cohen: some discussion whether there should be a single credential schema possible per credential.
… or something else.
… i want to have one defined in the VCDM.
… but need to discuss what people think.

Dave Longley: we already have mechanism for starting a set.
… for the case where we have more than one thing in the data model.
… if we are saying if we want to support either way, ability to have multiple schemas in a VC, i would expect to use that mechanism.

Orie Steele: The current JSON-LD context does not define credentialSchema as a container... https://github.com/w3c/vc-data-model/blob/main/contexts/credentials/v2#L18.

Dave Longley: if not, if different credential schema types can become multitype schemas then i would expect that somebody creates a multi purpose mechanism.
… i don't know if we need to invent a new thing to do that.
… not sure about the arguments and what are tradeoffs?

Gabe Cohen: not exactly an accurate representation.
… arguing for keeping complexitiy in specific type specifications.
… that is the purpose of having types for defining processing logic.
… want to avoid complextiy in data model.

Orie Steele: agree with gabe.
… dave is aruging that json-ld expansion can handle all of that.
… how to deal with polymorphism.
… let's not do any json-ld processing.

Brent Zundel: if i understand correctly if multiple schemas are present and the processing rules that then apply that we limit schemas to a single schema differentiated by types, so processing rules for single schema can be defined by the type and we also recognize that multiple schemas can be developed by having a composed schema.
… fair summary?
… is this something we can come to some agreement?

Gabe Cohen: correct brent.

Dave Longley: i don't know where i am at on this issue in particular.
… json-ld is sort of red herring.
… json-ld processors will find out a way to handle that no matter what we put into the spec.

Orie Steele: +1 dlongley.

Dave Longley: whether or not schemas are used is up to verifier.
… we can keep the data model simple and we can still put specific rules in specific type spec.
… and then up to verifier if follows rule or not.

Orie Steele: We are basically saying what is legal value for credentialSchema... object, array string or all of them.

Orie Steele: Sounds like we don't need to say anything if the verifier just knows how to do JSON-LD processing.

Oliver Terbu: I am in favor of keeping complexity on the type side.

Dmitri Zagidulin: my initial thought was that multiple items in cred schema was initially confusing. but then realized that.
… easy to understand example is different schema languages where multiple schemas would make sense.

Dave Longley: +1 to dmitriz, multiple schema languages.

Dmitri Zagidulin: shacl schema or json schema or whatever schema language other come up with.

Gabe Cohen: dmitri, that can easily be handled within a type.

Dave Longley: decentralgabe: a new "multi" type will just need to be invented.

Dmitri Zagidulin: question is if we need to specify and or or semantic for that field.

Orie Steele: sure, so we say credentialSchema supports array, object or string... and let type handle multiple langauges?

Orie Steele: and the core data model does not comment on the complexity?

Gabe Cohen: yes, multi type -> complex rules in the data model.

Dmitri Zagidulin: Orie - that seems reasonable.

Dave Longley: data model doesn't need those rules regardless.

Orie Steele: seems like we are really just saying "the core data model is JSON-LD" with extra words.

Brent Zundel: encourage folks to have that conversation.

Dmitri Zagidulin: decentralgabe - re "multiple schemas can be handled within a type" -- yeah, that's true too.

Brent Zundel: we would like to have this PR to move forward.

@brentzundel
Copy link
Member

After today's call I'm ready to close this PR and move back to discussion in the issue #950

Do you think that's reasonable - @dlongley @dmitrizagidulin @OR13 ?

I going to mark this as pending close and close in one week if no one objects.

@brentzundel brentzundel added pending close Close if no objection within 7 days and removed DO NOT MERGE PR contains something that should not be merged. discuss labels May 3, 2023
Copy link
Contributor

@OR13 OR13 left a comment

Choose a reason for hiding this comment

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

This PR has conflicts and based on the discussion in the last working group call, it seems better to just let JSON-LD norms build how this property can be used.

This means the following will all remain valid:

"credentialSchema": "IRI",
"credentialSchema": [ { "id": "IRI", "type": "Class", ...}, { "id": "IRI-2", "type": "Class-2", ...} ],
"credentialSchema": { "id": "IRI", "type": "Class", ...},

@decentralgabe
Copy link
Contributor Author

@OR13 I'm open to this change but then I believe we need to add language such as

If multiple credential schema types are provided they must be validated in order. Any single failure results in a validation failure for the credentialSchema property.

@OR13
Copy link
Contributor

OR13 commented May 5, 2023

@decentralgabe we don't need to add that language, because it's already assumed people understand how JSON-LD works.

We say the base media type is JSON-LD and the the standard uses compact form.

If people don't understand what that means, they cannot implement the spec.

If they understand it, they can do anything that is legal to a parser that processes the contexts

@OR13
Copy link
Contributor

OR13 commented May 5, 2023

Also member order is never preserved in JSON-LD, so there is no way to assure the order without adding additional details to the context.

@decentralgabe
Copy link
Contributor Author

@OR13 I'm not sure I fully understand how JSON-LD works. If we cannot guarantee processing order, will we still need to state how to interpret the results of processing multiple credential schemes? If not - how will we make sure verifiers have consistent results?

@iherman
Copy link
Member

iherman commented May 5, 2023

This PR has conflicts and based on the discussion in the last working group call, it seems better to just let JSON-LD norms build how this property can be used.

This means the following will all remain valid:

"credentialSchema": "IRI",
"credentialSchema": [ { "id": "IRI", "type": "Class", ...}, { "id": "IRI-2", "type": "Class-2", ...} ],
"credentialSchema": { "id": "IRI", "type": "Class", ...},

While this works and is correct, I am not sure it is the best choice. The usage of type has a very specific meaning in JSON-LD, and would require, if we play by the rules, to define a unique URL (or add it to one of our vocabularies) for Class, Class-2, etc. This is doable, but sounds like an overkill to me.

We may, instead, use a different key there (schema_type?), which is a property we can define and we could say that it has some predefine string values (json schema, json type definition, shacl, etc).

@iherman
Copy link
Member

iherman commented May 5, 2023

@OR13 I'm open to this change but then I believe we need to add language such as

If multiple credential schema types are provided they must be validated in order. Any single failure results in a validation failure for the credentialSchema property.

@decentralgabe we don't need to add that language, because it's already assumed people understand how JSON-LD works.

We say the base media type is JSON-LD and the the standard uses compact form.

If people don't understand what that means, they cannot implement the spec.

If they understand it, they can do anything that is legal to a parser that processes the contexts

I am not sure I understand this remark. JSON-LD is oblivious to this, which also means that it does not provide any guidance per se.

If credentialSchema is "just" used without further ado, the order of the schemas listed in the right hand side does not count. If credentialSchema is defined with range being a List (ie, it is defined, in JSON-LD terminology, as a list container) then the order counts. It is the specification's job to say what we want, then we can map the intention to JSON-LD.

(That being said, do we really want such a strong requirement, @decentralgabe? Alternatively, we could say that if any of the schema validate, we are good...)

@msporny
Copy link
Member

msporny commented May 5, 2023

@iherman wrote:

(That being said, do we really want such a strong requirement, @decentralgabe? Alternatively, we could say that if any of the schema validate, we are good...)

There is also the option that proof chains took in the Data Integrity spec via the previousProof property:

https://w3c.github.io/vc-data-integrity/#example-a-proof-chain-in-a-data-document

That said, don't know if anyone has such use cases today.

@decentralgabe
Copy link
Contributor Author

Closing since there isn't consensus. Let's please continue the discussion here #950 (comment)

@decentralgabe decentralgabe deleted the multiple-schemas-950 branch May 5, 2023 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending close Close if no objection within 7 days
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet