cleaner cleanup

rebase

The documentation should read pytest_plugins = ["swh.storage.pytest_plugin"]. You also need to add that line to your own conftest.py if you're depending on that fixture.

Please add a newline at the end of the file.

Please drop the line. Celery 4.4.4 isn't used anymore.

Oh, that needs a requirements-swh.txt bump as well, I guess.

Nice!

Apply some review comments

Reword

In D3428#84315, @swh-public-ci wrote:

Build has FAILED

That's pretty nice, thanks.

Looks nice.

I expect this has been fixed now...

Add test for get_listed_origins()

Drop limits checking

Add db upgrade script

That's great, thanks!

Thanks!

The change by itself looks fine, but the cursor manipulation logic is not sparking joy :-)

Accepted, with the following caveat: I'm still not sure what the plan is to deploy this now? In the current state of this diff, as soon as we deploy this on workers, data loss will occur (as the new extra_headers field isn't supported by swh.storage).

Reimplement copy_to escaping from the ground up.

I'm not sure about popping the extra_headers off of the incoming metadata field right now. This feels like something we want to do long term, but if you do that right now, it means you're putting upgrades of swh.model and swh.storage (with support with the new field) and all loaders in lockstep of one another. If you upgrade swh.model now, you'll lose the extra_headers until swh.storage will be able to store them.

Lazy loading for content objects (stored as files on the filesystem) gives us a choice where to put the cursor between the two ends of the eternal tradeoff between:

Maybe the asserts should be in an actual test function? Feels a bit wrong to have top-level asserts in that file.

(the suggested test should check that, for each ORIGIN_VISIT_STATUS, the date is > to the one of the matching ORIGIN_VISIT)