Re: [PATCH v2 05/10] bundle-uri: download in creationToken order
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 27, 2023, 19:32 UTC
- Message-ID
- <xmqqpmaz93k3.fsf@gitster.g>
- In-Reply-To
- <a2896d5b-f395-68df-1f23-356d0128cb9c@github.com>
Victoria Dye <vdye@github.com> writes:
Show 13 quoted lines
>> + /*
>> + * Not downloaded yet. Try downloading.
>> + *
>> + * Note that bundle->file is non-NULL if a download
>> + * was attempted, even if it failed to download.
>> + */
>> + if (fetch_bundle_uri_internal(ctx.r, bundle, ctx.depth + 1, ctx.list)) {
>> + /* Mark as unbundled so we do not retry. */
>> + bundle->unbundled = 1;
>
> This implicitly shows that, unlike a failed unbundling, a failed download is
> always erroneous behavior, with the added benefit of avoiding (potentially
> expensive) download re-attempts.Hmph, I somehow was hoping that we'd allow an option to use range requests to resume an interrupted download in the future, so outright "always avoid attempts to download again" may not be what we want in the longer run. But being able to tell if download failed (and there will probably be more than "success/failure" bit, but something like "we got an explicit 401 not found" vs "we were disconnected after downloading a few megabytes"), and unbundling failed (where there is no point attempting) is a good idea.
Show 15 quoted lines
>> cat >expect <<-EOF && >> - $HTTPD_URL/bundle-1.bundle >> - $HTTPD_URL/bundle-2.bundle >> - $HTTPD_URL/bundle-3.bundle >> + $HTTPD_URL/bundle-list >> $HTTPD_URL/bundle-4.bundle >> + $HTTPD_URL/bundle-3.bundle >> + $HTTPD_URL/bundle-2.bundle >> + $HTTPD_URL/bundle-1.bundle >> + EOF > > Ooh, interesting - using the new "test_remote_https_urls", these tests now > also verify that the bundles were downloaded in decreasing order when using > the 'creationToken' heuristic. That's a nice extra confirmation that the > heuristic is working as intended.
Yes, that indeed is very nice.