Page MenuHomeSoftware Heritage
Differential D1634

lister: Type correctly the 'indexable' column
ClosedPublic
Actions

Authored by ardumont on Jun 23 2019, 9:33 AM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Commits
rDLS3d473c307c1c: lister: Type correctly the 'indexable' column
Summary

instead of converting that column as a string

As a side effect, bitbucket wise, we provided improperly the after query
parameter as a date not url encoded. This resulted in improper api response from
bitbucket's (we received from time to time the same next index as the current
one).

Related T1826
Depends on D1632

Diff Detail

Repository
rDLS Listers
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.Jun 23 2019, 9:33 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptJun 23 2019, 9:33 AM
ardumont edited the summary of this revision. (Show Details)Jun 23 2019, 9:33 AM
ardumont added projects: Lister, Bitbucket lister.
ardumont added a child revision: D1635: indexing-lister: Allow to define flush packet size.Jun 23 2019, 9:36 AM
ardumont mentioned this in T1826: bitbucket lister does not work.
Harbormaster failed remote builds in B6428: Diff 5440!Jun 23 2019, 9:38 AM
swh-public-ci added a comment.Jun 23 2019, 9:38 AM

Build has FAILED

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

swh-public-ci added a comment.Jun 23 2019, 9:39 AM

Build has FAILED

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

ardumont updated this revision to Diff 5442.Jun 23 2019, 9:52 AM

Improve dosctrings and logging instruction

ardumont updated this revision to Diff 5444.Jun 23 2019, 9:55 AM

Update docstrings and logging instructions

(fix) previous one (my emacs did not update the hash after rebase... and i did
not see it immediately)

swh-public-ci added a comment.Jun 23 2019, 9:57 AM

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

Harbormaster completed remote builds in B6430: Diff 5442.Jun 23 2019, 9:57 AM
ardumont mentioned this in D1635: indexing-lister: Allow to define flush packet size.Jun 23 2019, 9:58 AM
Harbormaster failed remote builds in B6432: Diff 5444!Jun 23 2019, 9:58 AM
swh-public-ci added a comment.Jun 23 2019, 9:58 AM

Build has FAILED

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

ardumont planned changes to this revision.Jun 24 2019, 4:33 PM
ardumont updated this revision to Diff 5464.Jun 24 2019, 5:33 PM

bitbucket: Improve pagination api traversal in corner cases

Harbormaster failed remote builds in B6454: Diff 5464!Jun 24 2019, 5:33 PM
swh-public-ci added a comment.Jun 24 2019, 5:33 PM

Build has FAILED

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

ardumont updated this revision to Diff 5465.Jun 24 2019, 5:37 PM

Fix docstring

Harbormaster failed remote builds in B6455: Diff 5465!Jun 24 2019, 5:37 PM
swh-public-ci added a comment.Jun 24 2019, 5:38 PM

Build has FAILED

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

ardumont updated this revision to Diff 5466.Jun 24 2019, 5:49 PM

Amend query parameters shenanigans (keep the old behavior)

It made sense to change that behavior at some point but no longer.

Harbormaster failed remote builds in B6456: Diff 5466!Jun 24 2019, 5:50 PM
swh-public-ci added a comment.Jun 24 2019, 5:50 PM

Build has FAILED

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

ardumont edited the summary of this revision. (Show Details)Jun 24 2019, 5:52 PM
ardumont updated this revision to Diff 5474.Jun 25 2019, 1:13 PM

Update

Harbormaster failed remote builds in B6464: Diff 5474!Jun 25 2019, 1:13 PM
swh-public-ci added a comment.Jun 25 2019, 1:13 PM

Build has FAILED

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

douardda edited the summary of this revision. (Show Details)Jun 25 2019, 1:51 PM
douardda added a parent revision: D1633: bitbucket: Unify logging instructions.
swh-public-ci added a comment.Jun 25 2019, 1:57 PM

Build has FAILED

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

ardumont mentioned this in D1641: lister: Type correctly the 'indexable' column.Jun 25 2019, 2:30 PM
ardumont removed a child revision: D1635: indexing-lister: Allow to define flush packet size.Jun 25 2019, 2:37 PM
ardumont updated this revision to Diff 5485.Jun 25 2019, 2:38 PM

Rebase

ardumont edited the summary of this revision. (Show Details)Jun 25 2019, 2:38 PM
Harbormaster failed remote builds in B6475: Diff 5485!Jun 25 2019, 2:38 PM
swh-public-ci added a comment.Jun 25 2019, 2:39 PM

Build has FAILED

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

ardumont edited the summary of this revision. (Show Details)Jun 25 2019, 2:39 PM
ardumont added a parent revision: D1632: bitbucket: Allow incremental lister to start properly the first time.
ardumont removed a parent revision: D1633: bitbucket: Unify logging instructions.
swh-public-ci added a comment.Jun 25 2019, 2:57 PM

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

ardumont updated this revision to Diff 5489.Jun 25 2019, 2:59 PM

Rebase (tests ok)

ardumont retitled this revision from bitbucket: Improve pagination api traversal in corner cases to lister: Type correctly the 'indexable' column.Jun 25 2019, 2:59 PM
ardumont edited the summary of this revision. (Show Details)
swh-public-ci added a comment.Jun 25 2019, 3:13 PM

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

Harbormaster completed remote builds in B6479: Diff 5489.Jun 25 2019, 3:13 PM
vlorentz requested changes to this revision.Jun 26 2019, 10:42 AM
vlorentz added a subscriber: vlorentz.

indexable_type does not seem to be needed as a class attribute; and I'm not a fan of the name.

What about just a simple convert_type function, like you did for the test BB lister?
And also do it for the actual BB lister (it uses iso8601.parse_date twice)

swh/lister/bitbucket/tests/test_bb_lister.py
35

(I'm asking in another comment later to remove it, but FYI I don't think you need the staticmethod() call here)

This revision now requires changes to proceed.Jun 26 2019, 10:42 AM
vlorentz added inline comments.Jun 26 2019, 10:46 AM
swh/lister/bitbucket/tests/test_bb_lister.py
35

My bad, the method indeed gets bound to the class otherwise

ardumont added a comment.Jun 26 2019, 10:53 AM

indexable_type does not seem to be needed as a class attribute; and I'm not a fan of the name.

Yes, that name is inadequate.
It's in reference to the (indexer) listers using the column indexable (which is inadequate as well).
I did not want to push too far the fix though ;)

I'll rename the indexable_type to convert_type as suggested though.

ardumont updated this revision to Diff 5499.Jun 26 2019, 10:59 AM

Update according to review

swh-public-ci added a comment.Jun 26 2019, 11:03 AM

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

Harbormaster completed remote builds in B6489: Diff 5499.Jun 26 2019, 11:03 AM
vlorentz accepted this revision.Jun 26 2019, 11:17 AM
This revision is now accepted and ready to land.Jun 26 2019, 11:17 AM
Closed by commit rDLS3d473c307c1c: lister: Type correctly the 'indexable' column (authored by ardumont). · Explain WhyJun 26 2019, 11:19 AM
This revision was automatically updated to reflect the committed changes.
ardumont mentioned this in T1849: Deploy bitbucket lister.Jun 26 2019, 1:31 PM

Revision Contents
Changeset List

PathSize
swh/
lister/
bitbucket/
54 lines
4 lines
tests/
46 lines
core/
tests/
10 lines
github/
tests/
5 lines
gitlab/
tests/
5 lines
npm/
tests/
4 lines
phabricator/
tests/
1 line

Diff 5504

View Options

swh/lister/bitbucket/lister.py

Loading...
View Options

swh/lister/bitbucket/models.py

Loading...
View Options

swh/lister/bitbucket/tests/test_bb_lister.py

Loading...
View Options

swh/lister/core/tests/test_lister.py

Loading...
View Options

swh/lister/github/tests/test_gh_lister.py

Loading...
View Options

swh/lister/gitlab/tests/test_gitlab_lister.py

Loading...
View Options

swh/lister/npm/tests/test_npm_lister.py

Loading...
View Options

swh/lister/phabricator/tests/test_lister.py

Loading...
About · Developer information · Legal