Page MenuHomeSoftware Heritage

Add endpoint 'origin_list', that will replace 'origin_get_range'.
ClosedPublic

Authored by vlorentz on Nov 21 2019, 2:29 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

vlorentz created this revision.Nov 21 2019, 2:29 PM
ardumont requested changes to this revision.Dec 16 2019, 8:44 AM
ardumont added a subscriber: ardumont.
ardumont added inline comments.
swh/storage/in_memory.py
1160

Why don't you add type here and not within the docstring?

1165

I thought this was to be named count?

1170

retrieving

swh/storage/storage.py
1708

same remark than for in-memory implementation (type, name, typo).

1711

Why do we force the string type here?

1725

Why don't you change the db.origin_get_range implementation so that it retuns what you want instead?

This revision now requires changes to proceed.Dec 16 2019, 8:44 AM
vlorentz added inline comments.Dec 16 2019, 1:55 PM
swh/storage/in_memory.py
1160

Because I wrote that diff in mid-november and didn't touch it since; and we still used Python 3.5 at the time. I'll have to rebase it

1165

Yeah I guess we're not consistent here. I have a small preference for limit, sorry for asking the change on the other diff, I didn't realize the inconsistency

swh/storage/storage.py
1711

Because it's an opaque token, so clients shouldn't know what it actually contains so we can change it at any time. (eg. for Cassandra, it will be a large byte array)

1725

Because I need the origin id to compute the next_page_token.

vlorentz updated this revision to Diff 8693.Dec 16 2019, 2:49 PM

fix tests

vlorentz updated this revision to Diff 8699.Dec 16 2019, 3:16 PM

add type hints

vlorentz updated this revision to Diff 8700.Dec 16 2019, 3:17 PM

fix typo

vlorentz marked 3 inline comments as done.Dec 16 2019, 3:18 PM
ardumont added inline comments.Dec 16 2019, 3:48 PM
swh/storage/in_memory.py
1165

Well, i don't mind ;)

What's the conclusion though, shall we stay on limit or use count?
I ask because i'd have to change back in the other diff (uses count now).

ardumont accepted this revision.Dec 16 2019, 3:49 PM

Looks good

we need to decide whether we want to use count or limit.
We are not consistent yet.

(We are getting consistent on the page_token as an optional string though, yeah ;)

This revision is now accepted and ready to land.Dec 16 2019, 3:49 PM

Let's go for limit