Re: [PATCHv2 6/6] gitweb: check if-modified-since for feeds

From: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Date: Feb 6, 2009, 11:19 UTC
Message-ID: <cb7bb73a0902060319q474f3daeu5f67ea859cfa97b1@mail.gmail.com>
In-Reply-To: <200902050303.43356.jnareb@gmail.com>

On Thu, Feb 5, 2009 at 3:03 AM, Jakub Narebski <jnareb@gmail.com> wrote:

Show 15 quoted lines

> On Mon, 26 Jan 2009, Giuseppe Bilotta wrote:
>
>> Offering Last-modified header for feeds is only half the work, even if
>> we bail out early on HEAD requests. We should also check that same date
>> against If-modified-since, and bail out early with 304 Not Modified if
>> that's the case.
>
> It looks now quite nice, but I'd like to see information about
> dependencies for this feature in the commit message, something like:
>
>   This feature (terminating early with '304 Not Modified' in response
>   to 'If-Modified-Since' conditional request) requires having either
>   HTTP::Date module (from libwww-perl) or Time::ParseDate module.
>   If neither is present gitweb falls back to earlier behaviour of not
>   reacting to 'If-Modified-Since'.

Good idea.

Show 5 quoted lines

> Note also (although I'm not sure if it is worth mentioning in commit
> message) that it doesn't save gitweb as much work as one could think,
> because at this place the whole list of commits is already generated
> and parsed.  What we save is cost of running git-diff-tree for each
> commit (we could do better here, I think), and of course bandwidth.

If it's possible to get the first commit without generating and parsing the entire commit list, this can probably be optimized further.

> I wonder if it would be possible to separate this code into subroutine,
> to make it possible to have support for "cache control" conditional
> requests also in other cases where it might help (for the future of
> course, not in this commit).

Yes, I thought about it too.

Show 5 quoted lines

>
> Perhaps if we run gitweb from Apache mod_perl in compatibility mode
> (ModPerl::Registry) to use Apache Perl API to respond to
> 'If-Modified-Since' ($r->is_fresh or something). But that is also
> just idea for the future improvement.

Also, it's way beyond my current knowledge of Apache, CGI and Perl 8-D

-- 
Giuseppe "Oblomov" Bilotta