From: Jeff King <peff@peff.net>
Date: Tue, 10 Mar 2026 19:10:19 GMT
Subject: Re: [PATCH v5 0/4] http: add support for HTTP 429 rate limit retries
Message-ID: <20260310191019.GA589481@coredump.intra.peff.net>
In-Reply-To: <xmqq4imo1sse.fsf@gitster.g>

On Mon, Mar 09, 2026 at 04:34:25PM -0700, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > "Vaidas Pilkauskas via GitGitGadget" <gitgitgadget@gmail.com>
> > writes:
> >
> >> Changes since v4:
> >>
> >>  * fix only strbuf_attach() calls which don't need reallocation
> >>  * remove patch, which enforces strbuf_attach() contract via BUG()
> >> ...
> >> Vaidas Pilkauskas (4):
> >>   strbuf: pass correct alloc to strbuf_attach() in strbuf_reencode()
> >>   strbuf_attach: fix call sites to pass correct alloc
> >>   remote-curl: introduce show_http_message_fatal() helper
> >
> > These three patches looked quite reasonable to me.
> >
> >>   http: add support for HTTP 429 rate limit retries
> >
> > I'd feel comfortable to see somebody more familiar with the HTTP
> > transport code base to take a look at this step before we declare
> > victory.
> 
> Any volunteers?

Sorry, I'm way underwater on things I could/should be reviewing, and
this one was quite non-trivial. ;)

I just left some comments. A lot of it was about how to more cleanly
integrate with the http code (which I admit is a mess, especially with
respect to which "layer" things should happen at). Some of that may be
debatable, though I hope we can make things a bit cleaner.

But I think there may be a logic error in how http_request_recoverable()
loops, which certainly needs to be fixed.

-Peff