Page MenuHomeSoftware Heritage

swh.storage.buffer: Add buffering proxy storage implementation
ClosedPublic

Authored by ardumont on Tue, Oct 8, 2:46 PM.

Diff Detail

Repository
rDSTO Storage manager
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Tue, Oct 8, 2:46 PM
vlorentz requested changes to this revision.EditedTue, Oct 8, 3:00 PM

s/threshold/min_batch_size/

s/Sequence/Iterable/ because we don't do random access (see definition of sequences here: https://docs.python.org/3/glossary.html#term-sequence )

I think any call to a non-add method should trigger a flush of related objects before forwarding the call (eg. revision_missing flushes revisions).

swh/storage/buffer.py
21

.. code-block:: yaml

This revision now requires changes to proceed.Tue, Oct 8, 3:00 PM

Also, I understand how that's currently useful, but FYI, batching content has absolutely no effect for Cassandra (the Cassandra backend breaks batches into individual records)

Also, I understand how that's currently useful, but FYI, batching content has absolutely no effect for Cassandra (the Cassandra backend breaks batches into individual records)

that's the beauty of it!
That's used through configuration so we will be able to simply remove the configuration about the buffering storage when we migrate to cassandra!

swh/storage/buffer.py
21

I tried, that's too smart for me...

vlorentz added inline comments.Tue, Oct 8, 3:10 PM
swh/storage/tests/test_buffer.py
20

this should check the content was not sent to the backend.

Same comment for other instances below

24

len(content[0]['data']) + len(content[1]['data']).

Same comment for other instances below

ardumont updated this revision to Diff 7009.Tue, Oct 8, 3:54 PM
  • Fix code-block syntax
  • Add some more assertions on storage writing or not

BUILD has failed

./swh/storage/buffer.py:82:16: E999 SyntaxError: invalid syntax

Well i fixed my local mypy but the ci mypy (different version is not happy about it)...
That's annoying.

ardumont updated this revision to Diff 7010.Tue, Oct 8, 4:00 PM

Fix ci mypy which is not happy, local mypy was ok

ardumont updated this revision to Diff 7015.Tue, Oct 8, 4:16 PM

Rebase on latest development

vlorentz requested changes to this revision.Tue, Oct 8, 4:31 PM

You missed this comment:

s/threshold/min_batch_size/
s/Sequence/Iterable/ because we don't do random access (see definition of sequences here: https://docs.python.org/3/glossary.html#term-sequence )
I think any call to a non-add method should trigger a flush of related objects before forwarding the call (eg. revision_missing flushes revisions).

This revision now requires changes to proceed.Tue, Oct 8, 4:31 PM

You missed this comment:

I saw and then i forgot.

ardumont updated this revision to Diff 7016.Tue, Oct 8, 4:41 PM

Adapt according to last missing review points

This revision was not accepted when it landed; it landed in state Needs Review.Tue, Oct 8, 4:42 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.

This revision was not accepted when it landed; it landed in state Needs Review.

damn! I messed up.
i checked out one commit prior to my latest master... to push only to that level...
i git push-ed but that pushed my master anyway...

oh well, i'll start back using branch...