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
Branch
origin_list
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9788
Build 14453: tox-on-jenkinsJenkins
Build 14452: arc lint + arc unit

Event Timeline

ardumont added a subscriber: ardumont.
ardumont added inline comments.
swh/storage/in_memory.py
1159

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

1164

I thought this was to be named count?

1169

retrieving

swh/storage/storage.py
1707

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

1710

Why do we force the string type here?

1724

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
swh/storage/in_memory.py
1159

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

1164

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
1710

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)

1724

Because I need the origin id to compute the next_page_token.

swh/storage/in_memory.py
1164

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).

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