Vocabulary extensions #182

alien-mcl · 2019-01-30T20:04:40Z

Extended vocabulary with these features:

enable operations for non-RDF resources (resolves Allow returns/expects to be expressed in terms of a media type; what about multiple types? #22)
enable operations for headers (resolves Add support for describing headers accepted in supported operations #99)
enable operations with explicit operation target (API documentation) (resolves Expressing actions (#154) within ApiDocumentation #160, obsoletes Actions with explicit target #154 )
enable strongly typed collections (API documentation) (resolves API documentation limitations #167)
enable client-side initiated pagination (resoles Define client-initiated pagination #102)

In general, these imperfect attempt to extend the Hydra Core Vocabulary (HCV) should enable the it for some of the uses cases that appeared on various occasions. Feel free to deliberate more, but I hope these will find their way to the final specification.

I tried to keep backward compatibility so current implementations using the vocabulary in it's current state won't get broken. Unfortunately, there are some minor changes to some of the elements.

I've also added an updated version of the vocabulary.png diagram (including source code).

I'll try to provide a proper PR for Heracles.ts reference client covering these features ASAP.

- enable operations for non-RDF resources - enable operations for headers - enable operations with explicit operation target (API documentation) - enable strongly typed collections (API documentation) - enable client-side initiated pagination

angelo-v · 2019-02-05T09:34:23Z

spec/latest/core/core.jsonld

+      "vs:term_status": "testing"
+    },
+    {
+      "@id": "hydra:name",


hydra:name sounds very generic, as if you could name anything. Only the comment states, it is used to name a header. I would prefer hydra:headerName

Good point. Here you go!

alien-mcl · 2019-02-07T21:19:10Z

I've just pushed PR to Heracles.ts with support for these features.

tpluscode

I strongly disapprove of this pull request!

Please check my comments to individual pieces but there are bigger problems:

looks like there are several unrelated changes
none of the proposed changes have a related (or mentioned) PR and have not gone through some analysis beforehand
it literally came out of the blue

Instead of haphazardly submitting PRs we must follow a more structured workflow and follow some ground rules as closely as possible (I accept that there will be exceptions):

each PR should address a specific issue or at least related issues
each issue should be first discussed, deemed important and have a rough idea of the solution
PR with new features should not only introduce vocabulary elements but also extend upon the human-readable part of the spec.

I could go one step further and suggest that the prose should even come slightly before vocabulary. That way it could be easier to understand the intent and gauge the effect of said changes. On the other hand a discussion under the related issue would serve that purpose too.

tpluscode · 2019-02-09T13:58:53Z

spec/latest/core/core.jsonld

@@ -59,6 +59,19 @@
    "mapping": "hydra:mapping",
    "IriTemplateMapping": "hydra:IriTemplateMapping",
    "variable": "hydra:variable",
+    "skip": { "@id": "hydra:skip", "@type": "xsd:integer" },
+    "take": { "@id": "hydra:take", "@type": "xsd:integer" },


I think that skip/take are too specific go beyond Hydra Core. Sound more like Data than generic API description language.

I agree it's a desired feature but maybe this could became part of an extension focused on handling collections in different way?

If it is a desired feature, it should be provided in the core. I'm opened for alternative approaches though. These are just terms that are commonly used in other querying languages like SQL (TOP, LIMIT), LINQ (Skip, Take), etc.

tpluscode · 2019-02-09T14:55:17Z

spec/latest/core/core.jsonld

+      "@id": "hydra:target",
+      "@type": "rdf:Property",
+      "label": "invocation target",
+      "comment": "Explicit target of the invocation of the operation.",


Related to #154?

Yep, as it is stated in the PR's description at the very beginning

tpluscode · 2019-02-09T14:56:30Z

spec/latest/core/core.jsonld

+      "domain": "hydra:Operation",
+      "range": "xsd:string",
+      "vs:term_status": "testing"
+    },


How about possibleResponseHeader? That ways it's aligned with possibleStatus and HTTP terminology

I actually tried to be consistent with returns/expects. Indeed, for returned header possible may work. For expected it won't

tpluscode · 2019-02-09T14:56:55Z

spec/latest/core/core.jsonld

+      "domain": "hydra:Operation",
+      "rangeIncludes": ["hydra:Header", "hydra:HeaderTemplate"],
+      "vs:term_status": "testing"
+    },


Ditto, I propose to rename to requestHeader for example

While I believe both returned and expected headers should be connected to each other conceptually, this name is OK for me. For returned it should be then responseHeader

tpluscode · 2019-02-09T14:58:00Z

spec/latest/core/core.jsonld

+      "label": "Template",
+      "comment": "Some abstract template.",
+      "vs:term_status": "testing"
+    },


This is way too vague. Why do you think we need an "abstract template"? Sound very much like OOP

I was for backward compatibility - I didn't want to introduce another term for template in headers, thus I had to revamp the domain of the existing one. I also didn't want to replace domain with other predicate, thus this is why the abstract

tpluscode · 2019-02-09T14:58:26Z

spec/latest/core/core.jsonld

+      "label": "Header template",
+      "comment": "Specification of the header template with variables to be replaced with actual values.",
+      "vs:term_status": "testing"
+    },


What is the practical need for this Header template?

I was just wondering i.e. about an Authorization header where you mark the authorization scheme and the secret following it. Maybe other vendor-specific headers would require this.
In general, this is indeed a feature covering exotic scenarios.

tpluscode · 2019-02-09T15:00:01Z

spec/latest/core/core.jsonld

+      "@id": "hydra:Header",
+      "@type": "rdfs:DataType",
+      "label": "Header",
+      "comment": "Specification of the header, including name and value(s).",


I wonder, what is the current stand on Hydra vs HTTP?

I mean, adding headers to the core signifies a close relation with the protocol. Did we ever decide to keep Hydra somewhat agnostic in that regard?

@RubenVerborgh @lanthaler

There are possibleStatus and method - these are already quite strictly bound to the protocol. Also headers are not HTTP specific - other protocols may have similar notion. SOAP is a bad example here (as it can be over HTTP), but the envelope has a notion of the header as well.

Point taken

tpluscode · 2019-02-09T15:01:15Z

spec/latest/core/core.jsonld

+      "label": "Collection specification",
+      "comment": "Describes a collection returned by the operation.",
+      "vs:term_status": "testing"
+    },


Where does that come from? How would this come up against the "manages block"? Sounds similar

Ther was an issue on denoting on the API what the collection describes. This was an attempt of enabling that part of the spec with such a description.

tpluscode · 2019-02-09T15:01:42Z

spec/latest/core/core.jsonld

+      "label": "Media type",
+      "comment": "An RFC 6838 compliant media type specification.",
+      "vs:term_status": "testing"
+    },


The media type stuff is cool in principle but how does it relate to the other changes within this PR?

I wanted to enable hydra to expect/return resources other than RDF. One of the approach of describing a resource would be to use the media type, i.e. for JPG or DOCX file that would be expected by the API

alien-mcl · 2019-02-09T17:20:10Z

I strongly disapprove of this pull request!

Ok. Indeed this is an imperfect attempt.

looks like there are several unrelated changes

Indeed - there are several topics onboard

none of the proposed changes have a related (or mentioned) PR and
have not gone through some analysis beforehand

I partially disagree - operations with explicit targets were discussed quite extensively; other thing it didn't end up with any specific conclusion

it literally came out of the blue

I disagree - most of topics touched here were either discussed or at least mentioned; I also did advertised this attempt in various occasions

Instead of haphazardly submitting PRs we must follow a more structured
workflow and follow some ground rules as closely as possible

What kind of structured workflow? I agree that we didn't provide any detailed roadmap for future work, but GH is full of unresolved issues that needs to be addressed and should be treated as that roadmap. I remember how we did work in last several years and look were we are now.
I'd like to take a more agile approach with fail-fast attempts. I prefer to provide imperfect but working prototype (look at this PR at Heracles.ts) than to plot and plan and have endless discussions of greatness of Ionian school over Milesian school.

each PR should address a specific issue or at least related issues

I can cherry-pick each implemented feature as a separate PR if that will work for you.

each issue should be first discussed, deemed important and have a rough idea of the solution

As I already said - some of the features were discussed - some of them indeed may need some discussion. This is why that PR came anyway - to start discussion on some real approaches. I don't want to raise new issues to address other issues - it's pointless.

PR with new features should not only introduce vocabulary elements
but also extend upon the human-readable part of the spec.

I may agree with this, but this would imply enormous effort on me still having a large risk of a complete failure. Once I confirm that these changes are going in the right direction, I may consider spec changes.

I could go one step further and suggest that the prose should even come slightly before vocabulary.
That way it could be easier to understand the intent and gauge the effect of said changes.
On the other hand a discussion under the related issue would serve that purpose too.

Also a discussion on some real implementation still serves that purpose. Cherry-picking each feature with some brief would improve how these changes should be understood.

tpluscode · 2019-02-10T09:58:13Z

Without going into detailed discussions here I think this PR should be split into 3 PRs, which address the three linked issues it aims to resolve. The other two bullets should (non-RDF payloads and headers) should be first discussed as issues.

Splitting into smaller chunks will allow for focused discussions and prevent one change block an unrelated one from being merged.

On a technical note, it's best to avoid submitting PR from downstream master branch because such can cause conflicts once it's merged.

alien-mcl · 2019-02-11T18:53:48Z

Yep - I've just noticed master branch. One more reason to split it. As for new features - I'll try to dig through GH to find related issues. I'm pretty sure headers were mentioned somewhere.

alien-mcl · 2019-02-11T20:35:23Z

I've divided it into separate PRs (#183, #184, #185 and #186) - this PR is obsoleted by those.

alien-mcl added 2 commits January 30, 2019 20:46

Extended vocabulary with these features:

e7bd0f3

- enable operations for non-RDF resources - enable operations for headers - enable operations with explicit operation target (API documentation) - enable strongly typed collections (API documentation) - enable client-side initiated pagination

Added some missing items to context.

1f3adab

alien-mcl added design Operation Collection ApiDocumentation labels Jan 30, 2019

alien-mcl requested a review from RubenVerborgh January 30, 2019 20:04

angelo-v reviewed Feb 5, 2019

View reviewed changes

Replaced predicate name with headerName. Updated diagram

61defe8

tpluscode suggested changes Feb 9, 2019

View reviewed changes

This was referenced Feb 11, 2019

API documentation limitations #183

Closed

Define client-initiated pagination #184

Merged

Add support for describing headers #185

Merged

Allow returns/expects to be expressed in terms of a media type #186

Merged

alien-mcl closed this Feb 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vocabulary extensions #182

Vocabulary extensions #182

alien-mcl commented Jan 30, 2019 •

edited

Loading

angelo-v Feb 5, 2019

alien-mcl Feb 5, 2019

alien-mcl commented Feb 7, 2019

tpluscode left a comment

tpluscode Feb 9, 2019

alien-mcl Feb 9, 2019

tpluscode Feb 9, 2019

alien-mcl Feb 9, 2019

tpluscode Feb 9, 2019

alien-mcl Feb 9, 2019

tpluscode Feb 9, 2019

alien-mcl Feb 9, 2019

tpluscode Feb 9, 2019

alien-mcl Feb 9, 2019

tpluscode Feb 9, 2019

alien-mcl Feb 9, 2019

tpluscode Feb 9, 2019

alien-mcl Feb 9, 2019

tpluscode Feb 10, 2019

tpluscode Feb 9, 2019

alien-mcl Feb 9, 2019

tpluscode Feb 9, 2019

alien-mcl Feb 9, 2019

alien-mcl commented Feb 9, 2019 •

edited

Loading

tpluscode commented Feb 10, 2019

alien-mcl commented Feb 11, 2019

alien-mcl commented Feb 11, 2019

Vocabulary extensions #182

Vocabulary extensions #182

Conversation

alien-mcl commented Jan 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alien-mcl commented Feb 7, 2019

tpluscode left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alien-mcl commented Feb 9, 2019 • edited Loading

tpluscode commented Feb 10, 2019

alien-mcl commented Feb 11, 2019

alien-mcl commented Feb 11, 2019

alien-mcl commented Jan 30, 2019 •

edited

Loading

alien-mcl commented Feb 9, 2019 •

edited

Loading