Closed drernie closed 2 weeks ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 37.68%. Comparing base (
aa05544
) to head (c1ec361
). Report is 3 commits behind head on master.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@fiskus @sir-sigurd Any thoughts? I'm sure @nl0 will want to weigh in, but I'd appreciate your feedback before then.
Would it help to make the catalog part of the query string instead of the fragment?
quilt+s3://bkt?catalog=dns.name#package=pre/suf
On Mon, Oct 28, 2024 at 10:23 Maksim Chervonnyi @.***> wrote:
@.**** commented on this pull request.
In docs/Catalog/URI.md https://github.com/quiltdata/quilt/pull/4204#discussion_r1819460090:
+-
quilt+
: The scheme of the URI. This is alwaysquilt+
. +-s3://
: The protocol of the URI. This is currentlys3://
. +-<bucket>
: The name of the bucket containing the package or object, e.g.
quilt-example
. +-#package=<package>
: A fragment for the name of the package containing the- object, e.g.
akarve/cord19
.+In addition, it may contain the following optional components:
+-
<package>@<top_hash>
: The hash for this specific package, e.g.e21682f00929661879633a5128aaa27cc7bc1e2973d49d4c868a90f9fad9f34b
. +-<package>:tag
: The tag for this specific package. Currently, only thelatest
tag is supported. You may not specify both a top_hash and a tag. +-&path=<path>
: Fragment for the path to the object within the package, if- any, e.g.ß
CORD19.ipynb
. +-&catalog=<catalog>
: Fragment for the DNS name of catalog where this packageAlthough, I am not against adding the catalog parameter, I'm describing my doubts. It looks like, package, top_hash and path describe WHERE data is located. And the data is expected to be there no matter what, until S3 is online, and the user pays for it. On the other hand, catalog describes HOW to get that data. And lifespan of the catalog is different from S3. And it opens a question, should this type of information belong to URI.
- Basic HTTP auth, for example, is not a part of URI, it transmitted with HTTP Header
- But user/password for FTP is part of URI @.***)
- And, user/password for SSH too @.***)
So, considering this, I think, it's legit. But, maybe we can create separate scheme : for example, @. → @. It seems like overengineering, though. And barely readable
— Reply to this email directly, view it on GitHub https://github.com/quiltdata/quilt/pull/4204#pullrequestreview-2399732910, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAE2T6ZBOSOATCZTPA3WI3Z5ZQIFAVCNFSM6AAAAABQUHSKA6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGOJZG4ZTEOJRGA . You are receiving this because you authored the thread.Message ID: @.***>
quilt+s3://bkt?catalog=dns.name#package=pre/suf
I don't have a rational explanation, but that looks more elegant for me. I don't know (or don't remember), what was the rationale behind using #
hash instead of ?
query string in the first place.
I don't know (or don't remember), what was the rationale behind using
#
hash instead of?
query string in the first place.
AFAIR that was done because unlike fragment you need to encode querystring /
in particular
Documentation for
Propose new '&catalog` fragment.