Details

Reviewers

Group 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

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 6430
Build 8929: tox-on-jenkins	Jenkins
Build 8928: arc lint + arc unit

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

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

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

Improve dosctrings and logging instruction

Update docstrings and logging instructions

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

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

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

bitbucket: Improve pagination api traversal in corner cases

Harbormaster failed remote builds in B6454: Diff 5464!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

Fix docstring

Harbormaster failed remote builds in B6455: Diff 5465!Jun 24 2019, 5:37 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

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

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

Update

Harbormaster failed remote builds in B6464: Diff 5474!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.

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

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

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.

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

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)

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

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
32 ↗	(On Diff #5489)	(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
32 ↗	(On Diff #5489)	My bad, the method indeed gets bound to the class otherwise

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.