Re: [PULL 15/42] tests/functional: enable pre-emptive caching of assets

[Daniel P . Berrangé]

On Thu, Nov 28, 2024 at 10:54:43AM +0100, Thomas Huth wrote:
> On 27/11/2024 19.02, Richard Henderson wrote:
> > On 11/27/24 00:29, Thomas Huth wrote:
> > > On 26/11/2024 23.54, Richard Henderson wrote:
> > > > On 11/26/24 11:52, Thomas Huth wrote:
> > > > > I think we want to continue to maek failing downloads as
> > > > > test failures, otherwise we'll never notice when an asset is
> > > > > not available from the internet anymore (since SKIPs just
> > > > > get ignored).
> > > > 
> > > > I disagree.  Download failures are not rare.
> > > 
> > > That's not what I said / meant. Sure, servers can have hiccups and
> > > downloads can fail, but that's what we have the cache for. So having
> > > a working cache is essential.
> > > 
> > > OTOH, if you simply mark tests as SKIP if the download fail, we'll
> > > likely miss if an asset vanishes completely, since some people
> > > already have it in their cache and the remaining people will likely
> > > just ignore skipped tests.
> > If the cache is populated, we will *not* miss if an asset vanishes,
> > because we won't ever try the URL.
> 
> Well, we'll notice it as soon as people run the tests that don't have the
> asset in their cache yet.

We could make the cache validation logic do a "HEAD" request to
detect if the asset still exists, and fail on 404 even if we have
asset cached. The "HEAD" request should be generally fast given
it only grabs headers, no payload, unless the server is completely
DOSd.

> > If the cache is unpopulated, and the download fails, then we cannot run
> > the test. Indicating FAIL is *useless* because there's nothing that we
> > can do about it, and we also skip additional tests that CI could be
> > running.
> 
> Thinking about it for a little bit longer, I think we might rather want to
> distinguish the different failures that can occur during download. If we get
> a 404 error, it means that the asset has completely vanished and thus the
> test is broken, i.e. that's the case when we want to have a real error, I
> think.
> 
> But if the server is just not responding or gives a 5xx (like 503 or 504)
> server error, the failure is likely just a temporary one and we should skip
> the test instead. Does that sound acceptable to you? If so, I can look into
> creating a patch for this.



> 
>  Thomas
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|