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

Remove JSON Processing section #1298

Closed
wants to merge 1 commit into from
Closed

Conversation

OR13
Copy link
Contributor

@OR13 OR13 commented Oct 3, 2023

@OR13
Copy link
Contributor Author

OR13 commented Oct 3, 2023

I raised this PR, in an attempt to address the confusion being discussed regarding the JSON-LD processing section, as described in #1270 (comment)

I agree with @BigBlueHat

I think that there is never any "JSON Processing" of the core data model... There is just JSON-LD processing that is "light and unsafe" or "heavy and safer"... and implying that there is some how "safe JSON processing" is misleading, confusing and harmful to the work.

@dlongley
Copy link
Contributor

dlongley commented Oct 3, 2023

I think we should rename the section and apply whatever tweaks are needed rather than removing it. The content is absolutely valuable for helping people write code against data that is already expressed using a specific set of contexts they accept, which has great value. But this is not "JSON processing", whatever this means. "JSON processing" to me is just reading some JSON data and interpreting it according to the specs it is associated with -- which is what you need to do with any JSON, including JSON-LD.

The basic issue has been that newcomers to JSON-LD think there's something extra they need to do: that they need to change the data they have in their hand to be able to use it -- but they don't. If the data is already expressed using context(s) you accept, it's ready for use as-is. You can always do more if you want to, but this section covers ensuring you've got the data in the context(s) you want so you know you can use it simply.

@BigBlueHat
Copy link
Member

I'd agree with @dlongley that a rename of the section may be better than complete removal. However the JSON gets processed--be it with a "hand rolled" VC processor that only turns the JSON into an object in whatever programming language and does the basic validation checks described in this section OR a JSON-LD processor which is also constrained by the VCDM spec to limit what it "cares about" to the same set of contexts or similarly provided contexts (with validate-able hashes, etc.), the end result of those MUST be the same.

That said, a full JSON-LD processor provides more value beyond what is likely to be "hand rolled" by every developer writing unique code for processing the endless array of JSON document types out there. Full JSON-LD isn't a requirement, but certainly provides value beyond what's likely to be provided without one.

Some implementations even do both: quick/minimal checks at the JSON level (using JSON Schema you're writing, perhaps) and JSON-LD processing later to get the full richness of the graph and multilingual terms, storing and indexing them for faceted browsing/selection, or any number of non-trivial things one my want from the graph model that cannot be easily recreated if one were to rewrite all the JSON-LD richness again just for a limited VC implementation.

As @nissimsan put it in a recent email:

Echoing Orie, I would recommend a labeled property graph database. When working with Linked Data, you basically get this for free. Once there, you can jump right into Neo4j's Graph Data Science library.

So, there's value in both. The should remain possible and compatible, but full JSON-LD processing will always provide the richest possible experience because it's bringing more to the table since it's a syntax expanding on "just JSON" to enable all the things explained in #1270.

@OR13
Copy link
Contributor Author

OR13 commented Oct 4, 2023

We convert to N-Quads in order to compute Cypher... but we could just as easily compute the cypher from JSON or CBOR and it would cost less.

The RDF is actual harmful if it's crafted poorly and you assume it provides a "richer / better experience"... the most valuable feature of contexts is exploiting @id which makes the graph imported very different than JSON... meaning a light processor and a heavy one will come to wildly different conclusion at scale.

I think the working group is setting developers up for failure here... we should be more direct about limitations, and invite less unsafe behavior.

@BigBlueHat
Copy link
Member

BigBlueHat commented Oct 4, 2023

meaning a light processor and a heavy one will come to wildly different conclusion at scale.

I think the working group is setting developers up for failure here... we should be more direct about limitations, and invite less unsafe behavior.

@OR13 can you elaborate on what you mean here?

Copy link
Member

@BigBlueHat BigBlueHat left a comment

Choose a reason for hiding this comment

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

Let's keep the section and rename it to "Minimum Processing" or perhaps even "Minimum Viable Processing".

Copy link
Contributor

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

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

great improvement

@OR13
Copy link
Contributor Author

OR13 commented Oct 4, 2023

I'll attempt a PR that aligns both sections with what I think will be most helpful to implementers.

Copy link
Contributor

@awoie awoie left a comment

Choose a reason for hiding this comment

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

imo, it would help a lot if this section gets removed. it would solve #1290 as well.

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.

-1 to removing this section. Concrete suggestion, instead do what is suggested here: #1298 (comment) and here: #1298 (review)

@iherman
Copy link
Member

iherman commented Oct 12, 2023

The issue was discussed in a meeting on 2023-10-11

  • no resolutions were taken
View the transcript

1.7. Remove JSON Processing section (pr vc-data-model#1298)

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

Brent Zundel: This PR removes the "JSON processing" section. There's been some conversation. There has been request for changes.
… Looking for conversation here to see it move forward.

Dave Longley: This PR is an alternative to the one we discussed, I don't expect this PR to go in, if we get good consensus text on the other PR we just discussed. The other PR would be better for understanding what people would need to do.

Brent Zundel: If the other PR goes in, this one gets closed?

Dave Longley: That's my expectation.

@brentzundel brentzundel added the pending close Close if no objection within 7 days label Oct 17, 2023
@iherman
Copy link
Member

iherman commented Oct 17, 2023

The issue was discussed in a meeting on 2023-10-17

  • no resolutions were taken
View the transcript

2.8. Remove JSON Processing section (pr vc-data-model#1298)

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

Brent Zundel: We do not have consensus to merge 1298 and we will close it in favor of 1302.
… I will mark 1298 as pending close and we will close it when 1302.

Manu Sporny: +1 to close PR 1298 in favor of PR 1302.

Brent Zundel: Not we're going to talk about 1302.

@iherman
Copy link
Member

iherman commented Oct 31, 2023

The issue was discussed in a meeting on 2023-10-31

  • no resolutions were taken
View the transcript

1.7. Remove JSON Processing section (pr vc-data-model#1298)

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

Brent Zundel: This one is even more fraught. It's been marked pending close and I plan on closing after this meeting.
… This PR removes the entire JSON processing section.
… Any objections to closing after this meeting?

Manu Sporny: No objections to that.
… If you look across the various issues and threads -- there's some languaging going on, static vs. dynamic, application-specific vs. general processing, etc. There's something there that's all post-CR stuff that people could agree to. It would be nice to discuss that as a group, that might achieve consensus.
… Minor languaging changes might get us all into the same mental model and that could help the previously discussed PR changes easier to make.

@brentzundel
Copy link
Member

There is no consensus to make these changes, closing.

@brentzundel brentzundel closed this Nov 1, 2023
@msporny msporny deleted the remove-json-processing-section branch November 11, 2023 16:01
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

8 participants