I don't think this is useful anymore. The postgresql storage does the filtering itself now (see D2034). Are you planning on removing this feature from the postgresql storage?

swh/storage/filter.py
13	'This'

I don't think this is useful anymore. The postgresql storage does the filtering itself now (see D2034). Are you planning on removing this feature from the postgresql storage?

It always did that.
But it does it server-side so we send lots of data on the wire (which has a cost).

It's for doing it client side as the current loaders do but it's entangled within its own loading logic (check swh.loader.core.loader.BufferedLoader).

This allows to untangle that part configuration wise.

Maybe the issue is with where i've put the modules.
Maybe elsewhere swh.storage.client (or something) would be clearer...

swh/storage/filter.py
13	;)

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tox/671/
See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tox/671/console

BUILD has failed

swh/storage/filter.py:50: error: Argument 2 to "_filter_missing_ids" of "FilteringProxyStorage" has incompatible type "Generator[Any, None, None]"; expected "Sequence[bytes]"

hi ho!

In D2084#48356, @ardumont wrote:

I don't think this is useful anymore. The postgresql storage does the filtering itself now (see D2034). Are you planning on removing this feature from the postgresql storage?

It always did that.

Right, indeed

But it does it server-side so we send lots of data on the wire (which has a cost).

It's for doing it client side as the current loaders do but it's entangled within its own loading logic (check swh.loader.core.loader.BufferedLoader).

This allows to untangle that part configuration wise.

Right!

Accepting the diff, just make sure to fix the typo and the build

This revision is now accepted and ready to land.Oct 8 2019, 3:05 PM

Actually, I still have some nitpicking to do: instead of having sample_data as a large fixture returning a dict of various value types; could you split it into smaller fixtures, each returning a single type of values?

This revision now requires changes to proceed.Oct 8 2019, 3:07 PM

Accepting, because @ardumont promised to fix it on IRC ;)

This revision is now accepted and ready to land.Oct 8 2019, 3:13 PM

In D2084#48380, @vlorentz wrote:

Accepting, because @ardumont promised to fix it on IRC ;)

15:11 <+ardumont> i agree but there is an in-progress from douardda about the storage implem' tests
15:11 <+ardumont> and i think he will come up with something better
15:12 <vlorentz> are you *sure* this large fixture will be removed in the next ~2 weeks?
15:12 <+ardumont> most probably yes because *I*'ll make sure of it
15:12 <vlorentz> okay

Adapt according to review and mypy check

Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/673/ for more details.

Harbormaster completed remote builds in B8176: Diff 7006.Oct 8 2019, 3:40 PM

Fix commit message

Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/676/ for more details.

Harbormaster completed remote builds in B8181: Diff 7011.Oct 8 2019, 4:10 PM

Rebase to latest diff

Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/679/ for more details.

Harbormaster completed remote builds in B8184: Diff 7014.Oct 8 2019, 4:24 PM

Closed by commit rDSTOc83f1f9d0a7c: swh.storage.filter: Add filtering storage implementation (authored by ardumont). · Explain WhyOct 8 2019, 4:42 PM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDSTOc83f1f9d0a7c: swh.storage.filter: Add filtering storage implementation.