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 server requirement to advertise container type in HTTP header #190

Merged
merged 1 commit into from
Nov 1, 2021

Conversation

csarven
Copy link
Member

@csarven csarven commented Aug 6, 2020

Given that URI path ending with a slash denotes a container resource, server does not need to advertise the resource type in Link header. The initial and primary reason to include the container type was alignment with LDP, however this potential compatibility between a Solid server and a client that's only LDP conforming is not significant.

Edit: see also #191

@csarven csarven requested a review from a team August 6, 2020 12:29
@RubenVerborgh
Copy link
Contributor

I'm not sure how I feel about this, so will refrain from reviewing at the moment.

@csarven
Copy link
Member Author

csarven commented Aug 6, 2020

It is not required for interop, however I've raised alongside other basic resources types that a server may want to advertise: #191

Copy link
Member

@dmitrizagidulin dmitrizagidulin left a comment

Choose a reason for hiding this comment

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

Seems reasonable. 👍

@emmettownsend
Copy link

I don't like this at all.
I generally prefer being explicit and presenting a link header is being very explicit.
Depending on overloaded semantics in an identifier feels more implicit. Its also so much more open to developer error than an explicit header.
It is also inconsistent i.e. sometimes the resource type is denoted by a header and sometimes its not i.e. if it is a container.

@csarven
Copy link
Member Author

csarven commented Aug 27, 2020

The header is redundant. The other criteria (eg containment through slashes) is stronger; MUST for interop, otherwise everything falls apart. As said, the header can be useful but should be acknowledged as a separate category of concerns (issue 191). The spec doesn't forbid their use - so, optional, or at least can be considered as a good practice or convention to advertise resource types uniformly (where applicable).

@csarven csarven requested a review from a team September 16, 2020 12:54
@RubenVerborgh
Copy link
Contributor

My bigger issue here is: why would we intentionally drop LDP compliance? It's easier to say "Solid is LDP compliant" than "Solid follows LDP but…".

At the moment we're dropping one part of LDP compatibility, we're not LDP compatible, and therefore there would not be many reasons left to try for partial compability. In other words, if LDP compliance is not a goal, why are we even looking at LDP?

@RubenVerborgh
Copy link
Contributor

…but continuing on that though, wouldn't the line be redundant then? So we can remove it indeed—not because servers don't need to do it (they do), but because LDP already mandates that?

Base automatically changed from master to main March 4, 2021 21:40
Base automatically changed from main to master March 5, 2021 15:07
Base automatically changed from master to main March 5, 2021 15:11
@csarven csarven added this to the ~First Public Working Draft milestone Jun 10, 2021
Copy link
Member

@kjetilk kjetilk left a comment

Choose a reason for hiding this comment

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

I'm in favour.

I don't keep all problems at the top of my head anymore, but the assumption that we should follow LDP orthodoxy under all circumstances is something that has held us back for days and weeks.

LDP tries but fails to be a coherent structure, which means that you cannot really pick and choose if you say that you're accepting it as a normative basis. Assuming that it does created all kinds of problems for us.

Moreover, it was never the intention with Solid that it should be entirely LDP based, it basically just took the ideas of container structure, and took most of the HTTP interactions, because that is mostly just HTTP anyway. But all the strict interaction models and all that, that wasn't really Solid.

Just to mention a recent issue that illustrates how badly LDP does things, we cannot really rely on Basic Containers...: #194

It should be easy to implement Solid on top of an LDP server, but it should also be easier to write a Solid server from scratch than to first build an LDP server.

Thus, we don't want to totally break with LDP, but we need to relax some of LDPs constraints, Solid would basically not work with all those constraints anyway. This issue is simply one such relaxation that is in line with this.

@TallTed
Copy link
Contributor

TallTed commented Jun 28, 2021

Just checking that I'm reading this right --

With this change, Solid server MAY advertise container type in HTTP response header, so LDP Server (which MUST advertise container type in HTTP header) MAY also be a Solid server?

This is just meant to make it (minimally) easier to implement a Solid server which is not also an LDP server?

And to effectively make LDP compliance a value-add when comparing Solid implementations?

OK. Resolve the merge conflicts and do it.

@kjetilk
Copy link
Member

kjetilk commented Jun 29, 2021

Yup, that's right, @TallTed !

@kjetilk kjetilk added the status: Nominated An issue that has been nominated for the next monthly milestone label Sep 14, 2021
@csarven csarven self-assigned this Oct 6, 2021
@csarven csarven force-pushed the feature/resourcetype branch from db0f70b to 4dc28bd Compare November 1, 2021 10:41
@csarven
Copy link
Member Author

csarven commented Nov 1, 2021

Orthogonality of specifications.

  • Interop: As per Solid Protocol, Link rel=type LDPC is not needed in the response header for interop.
  • Reusability: Solid servers that also want to conform to LDP's interaction model can do so without conflicting with the Solid Protocol.

Resolved the conflicts (reset, force push). Merging.

Once gain, I'll defer to #191 for broader view on advertising resource type (with no particular dependency on LDP's "interaction model").

@csarven csarven merged commit 5ed37be into main Nov 1, 2021
@csarven csarven deleted the feature/resourcetype branch November 30, 2021 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc: Protocol status: Nominated An issue that has been nominated for the next monthly milestone topic: resource access
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants