Verify sha before cache write

[vondele]

No description provided.

[vondele]

works, must be seen if this resolves https://tests.stockfishchess.org/actions?max_actions=1&action=&user=&text=validate&before=1720333358.645925&run_id=

where a corrupted net ends up in the cache. Not clear how this can happen, at least this check will exclude the download part of the issue

[vdbergh]

It should solve the issue. The request package may create corrupt downloads.

[vondele]

but can it do without triggering an error (i.e. we call raise_for_status)?

Edit we do similar stuff for the zipballs we get from github, and somehow we can't as easily verify those.

Edit, we could probably try to unzip them.

[vdbergh]

I think it can although the mechanics are not clear to me. I suppose the library receives the correct http headers but then the download is aborted. But in principle the library should be able to verify that the correct number of bytes has been received.

[vdbergh]

Or maybe there is a bug in the error handling in the worker (I.e. an error is raised but not handled correctly). Can't check now. Traveling.

[vdbergh]

It seems older versions of the requests package did not check content length. Could it be that the requests package in the worker is outdated?

[vondele]

__version__ = '2.25.1' is in our repo

[ppigazzini]

That request version was the last compatible one with python 3.6.8 used by noob's workers.
However, local installed packages should still have the priority respect to the shipped packages (expression should be the only exception)

fishtest/worker/worker.py

Lines 30 to 33 in 12981ff

	# Fall back to the provided packages if missing in the local system.
	packages_dir = Path(__file__).resolve().parent / "packages"
	sys.path.append(str(packages_dir))

[vdbergh]

Last version on pypi is 2.32.3.

[vondele]

psf/requests#4956

so yeah, I think we're running into this.

[vdbergh]

Of course we could check it ourselves since we are using our own wrapper.

…

On Sun, Jul 7, 2024, 12:37 PM Joost VandeVondele ***@***.***> wrote: psf/requests#4956 <psf/requests#4956> so yeah, I think we're running into this. — Reply to this email directly, view it on GitHub <#2112 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABSOQ34LRN7ZMZJHOI7HATTZLELADAVCNFSM6AAAAABKPEOQG2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMJSGQYDEOJUHE> . You are receiving this because you commented.Message ID: ***@***.***>

[vondele]

looking into that... it is non-trivial.

[vondele]

giving up on checking the content-length, I don't get it to work (seems like it can depend on encoding, etc).

[vdbergh]

I am surprised by this... https://stackoverflow.com/questions/36194656/http-content-length-and-content-encoding

[vdbergh]

Ok I see. You cannot check it with the length of the downloaded file.

[vondele]

extend the original PR by moving the write of zipballs to after they have been unzipped and thus effectively been verified. Now both blobs written to cache are known to be OK.