Details

Reviewers

Group Reviewers

Commits

rDSTOd4fb270bbfe8: Move Storage documentation and endpoint paths to a new StorageInterface class

Summary

Documentation was duplicated between the in-mem and postgresql storage,
and one of them regularly goes out of date.
This deduplicates them both to a new class.

This new class is also the one declaring the API paths, as it did not
make sense to have this declaration on the postgresql storage.

Last but not least, this commit adds a test that checks backend classes
have all the functions, and they have exactly the same signature as the
interface. This will catch stupid bugs before production, eg. if an argument
does not have the same name in all classes.

Depends on D2583, D2584, D2586.

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

vlorentz created this revision.Jan 24 2020, 5:53 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptJan 24 2020, 5:53 PM

Harbormaster failed remote builds in B10283: Diff 9222!Jan 24 2020, 5:53 PM

Build has FAILED

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

Remove forgotten docstrings

Harbormaster failed remote builds in B10285: Diff 9224!Jan 24 2020, 5:55 PM

Build has FAILED

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

vlorentz added a child revision: D2589: Add return type to get_storage..Jan 24 2020, 6:12 PM

I started the review and asked for docstring updates.
I was gonna ask to add types as well.
But i stopped.

I guess we'd be better off go and do that in another dedicated and less big diff.

swh/storage/interface.py
15	what's the difference with `pass` keyword?
23	Remove that note and mark the method idempotent in the docstring. `Add content blobs to the storage idempotently` or something better if you have that in store.
44	remove that note as it's specific to the db implementation. Maybe move it there (don't know if we can do so now without overwriting all docstrings).
84	without objstorage insertion
swh/storage/tests/test_storage.py
104	thx for the comment!
106	why is this one aside? isn't it some left-overs from a tryout?

ardumont accepted this revision.Jan 27 2020, 9:47 AM

This revision is now accepted and ready to land.Jan 27 2020, 9:47 AM

ardumont added inline comments.Jan 27 2020, 11:10 AM

swh/storage/tests/test_storage.py
99	here.

ardumont mentioned this in D2586: Make db_transaction* remove db/cur from the signature..Jan 27 2020, 11:11 AM

vlorentz added inline comments.Jan 27 2020, 1:19 PM

swh/storage/tests/test_storage.py
106	I'm using a bit of magic, so I added an assertion to check the magic doesn't remove all the declared methods from `dir()`.

apply comment on content_add.

swh/storage/interface.py
15	Semantically, not much. But it's what mypy docs use to define protocols: https://mypy.readthedocs.io/en/stable/protocols.html#simple-user-defined-protocols
23	why? It is true of any backend.
44	indeed
84	what's the difference? and I did not write this btw, it's just copy-pasted