Re: [PATCH v2 05/10] bundle-uri: download in creationToken order

[Junio C Hamano <gitster@pobox.com>]

Victoria Dye <vdye@github.com> writes:

>> +			/*
>> +			 * 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.

>>  	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.