Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 HTTP spec #508
add HTTP spec #508
Changes from 30 commits
bc1aa59
1f075f6
12f86b8
146c09a
5398f5d
b6c1bc2
8a57943
d506145
946f516
3681472
dd5d07c
46d1857
ebe612c
7e5a077
db2b3b5
6319458
c7c9c43
454e25c
a25267b
3014b22
f96359b
1e87960
d0f0d93
8fbd64a
71415b0
4a03bb0
877899d
dc71f2c
d8850aa
78e8ca1
d30efda
8628b5a
3c0ac40
75bc635
f95e4db
e3eb9dc
95ffe6d
8f44d00
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
h2c to show how it can be multiplexed and negociated in many ways (header compression, binary based protocol, ...) ?
Feel free to ignore this comment if you think it's making the graph too complex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this is a bit confusing to me. Above you are saying the you generally refer to HTTP semantics and the next sentence says that a goal is to interoperate with existing HTTP servers and clients which refers to the transport, correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refers to both actually
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use well-known resources elsewhere, e.g.
.well-known/libp2p-webtransport
. Perhaps we should namespace this one too?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aesthetically it's nicer if we aren't referencing HTTP multiple times.
.well-known/libp2p
is a HTTP resource. It's kind of like saying "ATM machine".Technically this doesn't matter, but my vote is for
.well-known/libp2p
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I agree. Given we have other well-known resources,
.well-known/libp2p
is ambiguous,.well-known/libp2p-http
is not.Though yes, from a technical perspective it just has to be a predictable string with a low chance of collision.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lidel can you be our tie breaker?
.well-known/libp2p
vs.well-known/libp2p-http
or something elseThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Squatting
.well-known/libp2p
for a file may cause us problems in the future if we need to add something else.I think if we want to avoid that, there is still time to can change it (this spec is still a draft), we should go with either
.well-known/libp2p-http
..or make a directory and put protocol mapping configuration in.well-known/libp2p/http
.The latter (
libp2p/foo
) feels a bit better to me, as it creates libp2p-specific directory/namespace, which is easier to map via reverse proxies, but other than that it is mostly aesthetics.If we don't want 'http' twice, could be
.well-known/libp2p/protocols
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is different though. This is metadata about a peer's supported protocols. The well-know webtransport URI is about where to send the HTTP CONNECT request to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure it is, I think how you interact with the well-known resource is a detail of the protocol that's unrelated to the path it uses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
.well-known/libp2p/
suffix has the benefit of being a single thing we would need to register. Future versions of libp2p-webtransport may be placed under that suffix. Remember WebTransport isn't even out of draft status yet. We'll probably need to make a new multiaddr for webtransport-v1 just like we did with QUIC.Any other well-known resource we would want could also fit under that suffix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing this from "/.well-known/libp2p" to "/.well-known/libp2p/protocols" will break the ability to communicate with existing servers deployed without upgrading them first. Since these servers are running in places not under our control, they will not be immediately upgradable.
Because there are already deployments that use the old value, I suggest keeping it as
/.well-known/libp2p
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the pain in this rollout. Especially when the full rollout is outside your control. Maybe go-libp2p can provide a transition period to help out?
Since this spec itself is in draft we need flexibility in how things work. As much as I prefer the
.well-known/libp2p
, I don't think the argument of "we should use it because we've deployed this draft" is a strong one. Deployments of drafts are extremely useful to get some experience, but we shouldn't ossify to those decisions because we already made them.Once this is merged, things will be stable and we won't break existing users. That's the guarantee that comes with merging this. And partly the reason why I haven't merged it yet. I want to build the js-libp2p side before merging and make sure that the interop works and is reasonable. I'm pretty close on that. Hoping to get it done by the end of the month :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, this only specifies the path but not the method (GET / POST) to use when accessing this protocol over HTTP and that's up to the specific protocol to define how to run it over HTTP?
If so, should we add this explainer in the spec?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm.. the methods will be specific to each protocol at each mount point, so not part of this spec?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. As I see it there are two sections in this document:
So should we mention it in the specs that a libp2p protocol supporting http transport should specify the http method and headers to be used for the protocol. For the path they can expose it via the wellknown endpoint
/.well-known/libp2p/protocols
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand. An application protocol would be built using HTTP semantics, and that protocol would then be able to run on libp2p streams or "standard" http transports like h2, h3.
What do you mean by:
This spec does not define how you would take an existing libp2p protocol and map it to HTTP semantics. That is best done by the specific protocol itself. But maybe I'm misunderstanding your point?