Page MenuHomeSoftware Heritage
Differential D2324

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

Authored by vlorentz on Nov 21 2019, 2:29 PM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T1912: Support origin pagination without origin ids
Commits
rDSTOC0f9431247984: Add endpoint 'origin_list', that will replace 'origin_get_range'.
rDSTO0f9431247984: Add endpoint 'origin_list', that will replace 'origin_get_range'.
Summary

And uses the new pagination scheme, instead of origin ids.

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

vlorentz created this revision.Nov 21 2019, 2:29 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptNov 21 2019, 2:29 PM
swh-public-ci added a comment.Nov 21 2019, 2:34 PM

Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/804/ for more details.

Harbormaster completed remote builds in B9272: Diff 8015.Nov 21 2019, 2:34 PM
vlorentz added a task: T1912: Support origin pagination without origin ids.Nov 21 2019, 3:15 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
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
vlorentz added inline comments.Dec 16 2019, 1:55 PM
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.

vlorentz updated this revision to Diff 8688.Dec 16 2019, 2:12 PM

rebase

swh-public-ci added a comment.Dec 16 2019, 2:14 PM

Build has FAILED

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

Harbormaster failed remote builds in B9776: Diff 8688!Dec 16 2019, 2:14 PM
vlorentz updated this revision to Diff 8693.Dec 16 2019, 2:49 PM

fix tests

swh-public-ci added a comment.Dec 16 2019, 2:52 PM

Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/838/ for more details.

Harbormaster completed remote builds in B9781: Diff 8693.Dec 16 2019, 2:52 PM
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
swh-public-ci added a comment.Dec 16 2019, 3:19 PM

Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/839/ for more details.

Harbormaster completed remote builds in B9787: Diff 8699.Dec 16 2019, 3:19 PM
swh-public-ci added a comment.Dec 16 2019, 3:22 PM

Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/840/ for more details.

Harbormaster completed remote builds in B9788: Diff 8700.Dec 16 2019, 3:22 PM
ardumont added inline comments.Dec 16 2019, 3:48 PM
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).

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
vlorentz added a comment.Dec 16 2019, 3:50 PM

Let's go for limit

Closed by commit rDSTO0f9431247984: Add endpoint 'origin_list', that will replace 'origin_get_range'. (authored by vlorentz). · Explain WhyDec 16 2019, 3:51 PM
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDSTO0f9431247984: Add endpoint 'origin_list', that will replace 'origin_get_range'..
vlorentz added a commit: rDSTOC0f9431247984: Add endpoint 'origin_list', that will replace 'origin_get_range'..Jan 16 2020, 5:22 PM

Revision Contents
Changeset List

PathSize
swh/
storage/
34 lines
41 lines
tests/
26 lines

Diff 8700

View Options

swh/storage/in_memory.py

Loading...
View Options

swh/storage/storage.py

Loading...
View Options

swh/storage/tests/test_storage.py

Loading...
About · Developer information · Legal