-
Notifications
You must be signed in to change notification settings - Fork 232
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
IPIP-288: TAR Gateway Response Format #288
Conversation
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.
@hacdias looks good!
Got small ask: we are adopting light RFC process (#286), and this is a very nice, small improvement we could use to test the process.
Do you mind copying RFC/0000-template.md
from #289 and adding it to this PR?
It can be literally one sentence per each section – we just want to start explaining value to the user when we make changes like this one. Here, the benefit to the user is clear, they can download the entire directory. Alternatives and Security could include concerns around .gz
variant, as noted in ipfs/kubo#9034
f14fe3a
to
fb5678f
Compare
@lidel done 😄 |
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.
Thank you @hacdias!
Will get back to this when I have more time, but quick feedback for now:
TAR will be under similar special-case as CAR (weak Etag, no guarantee of byte-for-byte reproducibility), so need to add it to sections about cache control headers:
- rename RFC in markdown and file paths and names to IPIP (IPIP 0001: Lightweight "RFC" Process for IPFS Specifications #289)
- mention TAR next to CAR in
Etag
(response header) - mention TAR next to CAR in
Cache-Control
(response header)
8af8309
to
8023280
Compare
@lidel made the updates you asked for. Let me know if there's anything that should be re-written or added. |
@hacdias mind resolving conflicts / rebasing on top of main? I'll be going over IPIPs next week 🙏 |
358049d
to
b69968f
Compare
b69968f
to
6db9b5e
Compare
@lidel done! |
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.
minor nits after reading through
2cd86b4
to
9334f5e
Compare
Co-authored-by: Marcin Rataj <lidel@lidel.org>
This comment was marked as outdated.
This comment was marked as outdated.
@lidel added remaining fixtures. Cleaned up the copy. |
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've discussed this during IPFS Implementers Sync today and ratified this.
Keeping this open in case we want to make any last-moment clarifications based on ipfs/kubo#9029 (will merge this spec after we land working code).
See "Security" section of IPIP-288 (ipfs/specs#288)
Implementation of IPIP-288 (ipfs/specs#288) Co-authored-by: Marcin Rataj <lidel@lidel.org>
68eeda3
to
8fe745a
Compare
Merging. If anyone is interested in trying it out, a working Implementation from ipfs/kubo#9029 will ship in Kubo 0.17-rc1. |
See "Security" section of IPIP-288 (ipfs/specs#288) This commit was moved from ipfs/go-ipfs-files@e8cf9a3
Implementation of IPIP-288 (ipfs/specs#288) Co-authored-by: Marcin Rataj <lidel@lidel.org> This commit was moved from ipfs/kubo@a210abd
Implementation of IPIP-288 (ipfs/specs#288) Co-authored-by: Marcin Rataj <lidel@lidel.org>
See ipfs/kubo#9029.
Test failures: see #317 (comment).