Details

Reviewers

olasd
vlorentz
douardda

Group Reviewers

Reviewers

Commits

rDSTOCc83f1f9d0a7c: swh.storage.filter: Add filtering storage implementation
rDSTOc83f1f9d0a7c: swh.storage.filter: Add filtering storage implementation

Summary

Also add a sample_data fixture to read default test data from.

Depends on D2083

Test Plan

tox

Diff Detail

Repository

rDSTO Storage manager

Branch

master

Lint

Lint Skipped

Unit

Unit Tests Skipped

Build Status

Buildable 8184
Build 11807: tox-on-jenkins	Jenkins
Build 11806: arc lint + arc unit

Event Timeline

ardumont created this revision.Oct 8 2019, 2:40 PM

Harbormaster failed remote builds in B8172: Diff 7002!Oct 8 2019, 2:40 PM

Build has FAILED

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

Build has FAILED

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

ardumont added a child revision: D2085: swh.storage.buffer: Add buffering proxy storage implementation.Oct 8 2019, 2:46 PM

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.