Page MenuHomeSoftware Heritage
Differential D5754

tuleap: initialise lister.
ClosedPublic
Actions

Authored by borisbaldassari on May 19 2021, 2:49 PM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T3334: Tuleap lister
Commits
rDLS04c0a50706e8: tuleap: initialise lister.
Required Signatures
L3 Software Heritage Contributor License Agreement, version 1.0
Summary

List all git origins from a tuleap instance.

Related to T3334

Test Plan

Retrieve list of projects (2) and get their repositories (2).

Diff Detail

Repository
rDLS Listers
Branch
T3334_tuleap_lister
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 21543
Build 33469: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 33468: arc lint + arc unit

Unit TestsFailed
View All

TimeTest
300 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.lister.tests.test_cli::test_get_lister
swh_scheduler_config = {'db': "dbname=scheduler user=postgres host=127.0.0.1 port=23950 options=''"} def test_get_lister(swh_scheduler_config):
1,004 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.lister.tuleap.tests.test_tasks::test_full_listing
__wrapped_mock_method__ = <function NonCallableMock.assert_called_with at 0x7f10fc6280d0> args = (<MagicMock name='TuleapLister.from_configfile' id='139710830902520'>,) kwargs = {'instance': None, 'url': 'https://tuleap.net'}
63 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.lister.bitbucket.tests.test_lister::test_bitbucket_full_lister
1,302 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.lister.bitbucket.tests.test_lister::test_bitbucket_incremental_lister
57 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.lister.bitbucket.tests.test_lister::test_bitbucket_lister_rate_limit_hit
View Full Test Results (2 Failed · 173 Passed)

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
ardumont edited the summary of this revision. (Show Details)May 19 2021, 3:00 PM
borisbaldassari updated this revision to Diff 20569.May 19 2021, 3:01 PM
  • tuleap-lister: fix args in test_task.
ardumont added a subscriber: ardumont.May 19 2021, 3:02 PM

Thanks.

@borisbaldassari You need to sign the document [1 as mentioned in the view.

Otherwise we won't be able to review the code, it's hidden until that ^ is signed ;)

[1] https://forge.softwareheritage.org/L3

swh-public-ci added a comment.May 19 2021, 3:05 PM

Build has FAILED

Patch application report for D5754 (id=20569)

Rebasing onto 8f3bbacd5e...

First, rewinding head to replay your work on top of it...
Applying: tuleap: initialise lister.
Applying: tuleap-lister: fix args in test_task.
Changes applied before test
commit 29b786cd69f2e4a0c105b040bdf436d7b1fcbfbb
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 15:00:32 2021 +0200

    tuleap-lister: fix args in test_task.

commit 6aceb4680d1a353ff198c1bf23ed13180038913e
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 14:44:55 2021 +0200

    tuleap: initialise lister.

Link to build: https://jenkins.softwareheritage.org/job/DLS/job/tests-on-diff/285/
See console output for more information: https://jenkins.softwareheritage.org/job/DLS/job/tests-on-diff/285/console

Harbormaster returned this revision to the author for changes because remote builds failed.May 19 2021, 3:05 PM
Harbormaster failed remote builds in B21543: Diff 20569!
borisbaldassari added a comment.EditedMay 19 2021, 4:12 PM

@borisbaldassari You need to sign the document [1 as mentioned in the view.

Thanks @ardumont for your attention. I've signed the doc (while you were typing this comment I guess).

Furthermore: jenkins helped me fix a couple of typos, that's cool, but still no luck with the job status (and the task tests). I've identified 2 errors:

  • TypeError: init() missing 1 required positional argument: 'url'

Yet no idea where I could tell tests to yield the url (everything I've checked looks fine, or similar to other successful lister tests)

  • 15:05:24 E AssertionError: Expected call: from_configfile(instance=None, url='https://tuleap.net')

15:05:24 E Actual call: from_configfile(url='https://tuleap.net')
Same here, no idea where this should be set. the code is exactly the same as for gitea, and as I understand it the instance=None *should* be there.

Any hint? Thanks a lot in advance (ramping up on swh is a bit tough, but sooo interesting!).

borisbaldassari added a comment.May 19 2021, 5:00 PM

Interestingly I can't reproduce the lister error (init missing reuqired parameter) on my own setup.
The error on the test_task is consistent with my own pytest/tox error though.

ardumont added a comment.EditedMay 19 2021, 5:08 PM

Interestingly I can't reproduce the lister error (init missing reuqired parameter) on my own setup.
The error on the test_task is consistent with my own pytest/tox error though.

Regarding the tasks module tests, It usually is something around the mocked imported module which is wrong for some reason.
Not that i see anything immediately wrong in your code.

swh/lister/tuleap/tasks.py
12

why Relister?

I think FullTuleapLister is fine enough (and that does not clash with the imported .lister.TupleLister class

16

you can drop the print when you are done ;)

ardumont added inline comments.May 19 2021, 5:10 PM
swh/lister/tuleap/tests/test_tasks.py
20

I do not exactly recall why but now we tend to use the mocker fixture instead of the @patch stanza [1]

Can you give it a try?

[1] https://forge.softwareheritage.org/source/swh-lister/browse/master/swh/lister/cgit/tests/test_tasks.py$0-17

ardumont added a comment.EditedMay 19 2021, 5:17 PM

TypeError: init() missing 1 required positional argument: 'url'
Yet no idea where I could tell tests to yield the url (everything I've checked looks fine, or similar to other successful lister tests)

15:05:24 E AssertionError: Expected call: from_configfile(instance=None, url='https://tuleap.net')

I figure it's because that test needs some extra step. The url is mandatory so you need
something like [1] to make it ok (it's not ideal though but that's the current test ;)

I added the following to the [1] dict and it passed:

"tuleap": {"url": "https://tuleap",},

[1] https://forge.softwareheritage.org/source/swh-lister/browse/master/swh/lister/tests/test_cli.py$0-10

ardumont added inline comments.May 19 2021, 5:28 PM
swh/lister/tuleap/tests/test_tasks.py
32–34

I got it green by modifying with the ^

You did pass the kwargs to the task (line 26), so that's what's passed to the lister.

borisbaldassari updated this revision to Diff 20572.May 19 2021, 5:38 PM
borisbaldassari marked an inline comment as done.
  • tuleap-lister: Add rate-limiting test + fix debug and typo.
swh-public-ci added a comment.May 19 2021, 5:40 PM

Build has FAILED

Patch application report for D5754 (id=20572)

Rebasing onto 8f3bbacd5e...

First, rewinding head to replay your work on top of it...
Applying: tuleap: initialise lister.
Applying: tuleap-lister: fix args in test_task.
Applying: tuleap-lister: Add rate-limiting test + fix debug and typo.
Changes applied before test
commit d57e4817ade90596a17dae6cad34516878823b5b
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:06:54 2021 +0200

    tuleap-lister: Add rate-limiting test + fix debug and typo.

commit 23b1aa982b979493e2e08de78f9c1227cf6c6b67
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 15:00:32 2021 +0200

    tuleap-lister: fix args in test_task.

commit e25712217930a3ee2742e40e5e13a55b3664a899
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 14:44:55 2021 +0200

    tuleap: initialise lister.

Link to build: https://jenkins.softwareheritage.org/job/DLS/job/tests-on-diff/286/
See console output for more information: https://jenkins.softwareheritage.org/job/DLS/job/tests-on-diff/286/console

Harbormaster returned this revision to the author for changes because remote builds failed.May 19 2021, 5:40 PM
Harbormaster failed remote builds in B21547: Diff 20572!
borisbaldassari updated this revision to Diff 20573.May 19 2021, 5:42 PM
  • tuleap-lister: code review: fix mocker + tests/setup_cli.
  • tuleap-lister: code review: fix relister > lister.
swh-public-ci added a comment.May 19 2021, 5:45 PM

Build has FAILED

Patch application report for D5754 (id=20573)

Rebasing onto 8f3bbacd5e...

First, rewinding head to replay your work on top of it...
Applying: tuleap: initialise lister.
Applying: tuleap-lister: fix args in test_task.
Applying: tuleap-lister: Add rate-limiting test + fix debug and typo.
Applying: tuleap-lister: code review: fix mocker + tests/setup_cli.
Applying: tuleap-lister: code review: fix relister > lister.
Changes applied before test
commit 9320db8e5dceafe87636a7d639d659c2a7efbfdd
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:41:43 2021 +0200

    tuleap-lister: code review: fix relister > lister.

commit bae17403957a22080e29fd31978fa7ecd632d680
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:39:24 2021 +0200

    tuleap-lister: code review: fix mocker + tests/setup_cli.

commit 171a9a2e730bb63d3e1b926f052866be8eac094e
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:06:54 2021 +0200

    tuleap-lister: Add rate-limiting test + fix debug and typo.

commit 589742e11c2a2a577c2852d9d66c646851ed8405
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 15:00:32 2021 +0200

    tuleap-lister: fix args in test_task.

commit cf65ec19cab72c6f27c69a0cc2c3c146fe2d3f5a
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 14:44:55 2021 +0200

    tuleap: initialise lister.

Link to build: https://jenkins.softwareheritage.org/job/DLS/job/tests-on-diff/287/
See console output for more information: https://jenkins.softwareheritage.org/job/DLS/job/tests-on-diff/287/console

Harbormaster returned this revision to the author for changes because remote builds failed.May 19 2021, 5:45 PM
Harbormaster failed remote builds in B21548: Diff 20573!
borisbaldassari updated this revision to Diff 20574.May 19 2021, 5:55 PM
  • tuleap-lister: code review: fix test_task kwargs.
swh-public-ci added a comment.May 19 2021, 5:58 PM

Build is green

Patch application report for D5754 (id=20574)

Rebasing onto 8f3bbacd5e...

First, rewinding head to replay your work on top of it...
Applying: tuleap: initialise lister.
Applying: tuleap-lister: fix args in test_task.
Applying: tuleap-lister: Add rate-limiting test + fix debug and typo.
Applying: tuleap-lister: code review: fix mocker + tests/setup_cli.
Applying: tuleap-lister: code review: fix relister > lister.
Applying: tuleap-lister: code review: fix test_task kwargs.
Changes applied before test
commit fa2e5f818ade622fee719eac89c6b75b51e56a62
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:52:36 2021 +0200

    tuleap-lister: code review: fix test_task kwargs.

commit 169abacee91b9ea4d4f5920b5ee68da24f58dfe0
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:41:43 2021 +0200

    tuleap-lister: code review: fix relister > lister.

commit 0d903ce69dccddf14e49b06d9fc21a3e5a91c3ba
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:39:24 2021 +0200

    tuleap-lister: code review: fix mocker + tests/setup_cli.

commit 9fd4ddd652df790032edfbb0ef96420ad631dda2
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:06:54 2021 +0200

    tuleap-lister: Add rate-limiting test + fix debug and typo.

commit e03c2e1cf3a29b947396a4853aa095ed19590dc1
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 15:00:32 2021 +0200

    tuleap-lister: fix args in test_task.

commit ad76aaa3bfd61d0d9a917c1b5bb4012895a86c8f
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 14:44:55 2021 +0200

    tuleap: initialise lister.

See https://jenkins.softwareheritage.org/job/DLS/job/tests-on-diff/288/ for more details.

Harbormaster completed remote builds in B21549: Diff 20574.May 19 2021, 5:58 PM
borisbaldassari requested review of this revision.May 19 2021, 5:58 PM
ardumont added inline comments.May 20 2021, 9:21 AM
swh/lister/tuleap/lister.py
83

I recall they describe the authentication per token deprecated.
And they mention something about custom headers [2].

So if it's indeed not the right implementation [3], it'd be fine if you provide a tuleap
lister implementation relying only on anonymous listing first. That means, you can drop
that credentials initialization part, but keep the credentials parameter in the
constructor (and in the super call) though (implementation detail which requires it).

We (including you) can always dig in later to improve the implementation so we can
actually connect to the tuleap instance (if that's needed, i don't know at which point
the anonymous listing is restricted).

[1] https://tuleap.net/doc/en/user-guide/integration/rest/quick-start/auth.html

[2] custom headers mentioned:

X-Auth-Token: value of token attribute received from /api/tokens
X-Auth-UserId: value of user attribute received from /api/tokens

[3] it's better if new code is tested. You can rely on the jenkins build to detect
what's amiss

ardumont added a comment.May 20 2021, 9:21 AM

I forgot to mention you should add yourself to the top-level CONTRIBUTORS file ;)

ardumont added inline comments.May 20 2021, 9:38 AM
swh/lister/tuleap/lister.py
106–111

If i understood the rtype correctly that is.

ardumont added inline comments.May 20 2021, 9:42 AM
swh/lister/tuleap/lister.py
114

please add type annotation, this helps reading the code ;)

vlorentz added a subscriber: vlorentz.May 20 2021, 1:38 PM

Hi, thanks for this.

A few general comments:

  • it looks like the new python files are made executable, but they shouldn't be, can you fix it? (chmod -X *.py)
  • Copyright date should be only 2021
  • does the original data for https_tuleap.net/projects and others have this weird indentation, or is this an artefact of your IDE?
borisbaldassari updated this revision to Diff 20598.May 20 2021, 3:08 PM
borisbaldassari marked 3 inline comments as done.
  • tuleap-lister: code review: Remove authentication useless lines + fix typos.
swh-public-ci added a comment.May 20 2021, 3:12 PM

Build is green

Patch application report for D5754 (id=20598)

Rebasing onto 8f3bbacd5e...

First, rewinding head to replay your work on top of it...
Applying: tuleap: initialise lister.
Applying: tuleap-lister: fix args in test_task.
Applying: tuleap-lister: Add rate-limiting test + fix debug and typo.
Applying: tuleap-lister: code review: fix mocker + tests/setup_cli.
Applying: tuleap-lister: code review: fix relister > lister.
Applying: tuleap-lister: code review: fix test_task kwargs.
Applying: tuleap-lister: code review: Remove authentication useless lines + fix typos.
Changes applied before test
commit db87711d77a25006693b7e9c91d7a8bf24670805
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 10:55:36 2021 +0200

    tuleap-lister: code review: Remove authentication useless lines + fix typos.

commit 8658da1623a4ac68b9fffdcbe3dda43af70ba471
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:52:36 2021 +0200

    tuleap-lister: code review: fix test_task kwargs.

commit fd9839b8623efd1fe7712b0468866a9a401b6ab1
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:41:43 2021 +0200

    tuleap-lister: code review: fix relister > lister.

commit 9a312a6f2b390eb81f75ce0f77d1d25a4f45891e
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:39:24 2021 +0200

    tuleap-lister: code review: fix mocker + tests/setup_cli.

commit 421cdc13cf7c9d1f85424e1a0e9e75642fa8a6b9
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:06:54 2021 +0200

    tuleap-lister: Add rate-limiting test + fix debug and typo.

commit 340abc6daf6aab448e02da8b9b2af52e25889bfc
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 15:00:32 2021 +0200

    tuleap-lister: fix args in test_task.

commit a5c4b3e2fbd5fafc8554e028f6dd3aa24ea55520
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 14:44:55 2021 +0200

    tuleap: initialise lister.

See https://jenkins.softwareheritage.org/job/DLS/job/tests-on-diff/289/ for more details.

Harbormaster completed remote builds in B21563: Diff 20598.May 20 2021, 3:12 PM
borisbaldassari updated this revision to Diff 20604.May 20 2021, 4:20 PM
borisbaldassari marked 3 inline comments as done.
  • tuleap-lister: code review: improve results_simplified for svn repos.
  • tuleap-lister: code review: add name to CONTRIBUTORS file.
  • tuleap-lister: code review: Update tutorial for misc files to edit.
swh-public-ci added a comment.May 20 2021, 4:23 PM

Build is green

Patch application report for D5754 (id=20604)

Rebasing onto 8f3bbacd5e...

First, rewinding head to replay your work on top of it...
Applying: tuleap: initialise lister.
Applying: tuleap-lister: fix args in test_task.
Applying: tuleap-lister: Add rate-limiting test + fix debug and typo.
Applying: tuleap-lister: code review: fix mocker + tests/setup_cli.
Applying: tuleap-lister: code review: fix relister > lister.
Applying: tuleap-lister: code review: fix test_task kwargs.
Applying: tuleap-lister: code review: Remove authentication useless lines + fix typos.
Applying: tuleap-lister: code review: improve results_simplified for svn repos.
Applying: tuleap-lister: code review: add name to CONTRIBUTORS file.
Applying: tuleap-lister: code review: Update tutorial for misc files to edit.
Changes applied before test
commit 43f5ffcc5524b3f828e8e2d7f574803ac6cf7cf5
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 16:18:40 2021 +0200

    tuleap-lister: code review: Update tutorial for misc files to edit.

commit aeb04c0ee28e7eea5e294cabc702e8ad8f77f64a
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 15:46:02 2021 +0200

    tuleap-lister: code review: add name to CONTRIBUTORS file.

commit 6ff122d869c693373a0cbdb9ea3cc158aba2ac26
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 15:43:07 2021 +0200

    tuleap-lister: code review: improve results_simplified for svn repos.

commit 43cf74520a70df4521e06e8d5b0362827da39219
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 10:55:36 2021 +0200

    tuleap-lister: code review: Remove authentication useless lines + fix typos.

commit 5a65eeb18afccab0c6e8d1b67dc187fc3ae913d7
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:52:36 2021 +0200

    tuleap-lister: code review: fix test_task kwargs.

commit 6d0ba20b6fd172132cbafddc2067299a2f2da1ab
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:41:43 2021 +0200

    tuleap-lister: code review: fix relister > lister.

commit b7a69a6ab18743834477ac320693036da90a6045
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:39:24 2021 +0200

    tuleap-lister: code review: fix mocker + tests/setup_cli.

commit 41897ddea0f5c686f2944b2e51d3ad5178516237
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:06:54 2021 +0200

    tuleap-lister: Add rate-limiting test + fix debug and typo.

commit d5efcfe4111ce5e14497077ae06574dc9ec53d8f
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 15:00:32 2021 +0200

    tuleap-lister: fix args in test_task.

commit 2193a25f4193db9240ed91b3bec5ad44414f9bb2
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 14:44:55 2021 +0200

    tuleap: initialise lister.

See https://jenkins.softwareheritage.org/job/DLS/job/tests-on-diff/290/ for more details.

Harbormaster completed remote builds in B21569: Diff 20604.May 20 2021, 4:24 PM
borisbaldassari added a comment.May 20 2021, 4:28 PM

Hi vlorentz, thanks for the inputs (and good shot for the copyrights + chmod!!)

Regarding the json, no it's returned as minimised json, but I thought it would be easier to human-read and debug this way. Quite pointless on 2nd thoughts. I'll remove it.

swh/lister/tuleap/lister.py
106–111

You did understand well. :-)
The svn part is not implemented yet, since I cannot test it - but that's in the "immediate roadmap". I've included your modification, since they will be needed soon.

106–111

On second thougths you almoost got it: the method is a classmethod, so self won't work. B

swh/lister/tuleap/tasks.py
12

No reason really, it's just that gitea had it, so I thought it would be ok to just leave it. Changed it to FullTuleapLister in the upcoming commit.

swh/lister/tuleap/tests/test_tasks.py
32–34

Hum.. I assumed this test would test the default value for instance, hence the diff. BTW the failing code is present in gitea, where all tests are green :/)

As a matter of fact you're right (yeaaah) tests pass with the change, so I'll push it now. Hope I'm not fixing the test without fixing the code...

borisbaldassari updated this revision to Diff 20605.May 20 2021, 4:34 PM
  • tuleap-lister: code review: Update copyright to 2021 exactly.
  • tuleap-lister: code review: Update py files perms -X.
  • tuleap-lister: code review: minimise json files.
swh-public-ci added a comment.May 20 2021, 4:39 PM

Build is green

Patch application report for D5754 (id=20605)

Rebasing onto 8f3bbacd5e...

First, rewinding head to replay your work on top of it...
Applying: tuleap: initialise lister.
Applying: tuleap-lister: fix args in test_task.
Applying: tuleap-lister: Add rate-limiting test + fix debug and typo.
Applying: tuleap-lister: code review: fix mocker + tests/setup_cli.
Applying: tuleap-lister: code review: fix relister > lister.
Applying: tuleap-lister: code review: fix test_task kwargs.
Applying: tuleap-lister: code review: Remove authentication useless lines + fix typos.
Applying: tuleap-lister: code review: improve results_simplified for svn repos.
Applying: tuleap-lister: code review: add name to CONTRIBUTORS file.
Applying: tuleap-lister: code review: Update tutorial for misc files to edit.
Applying: tuleap-lister: code review: Update copyright to 2021 exactly.
Applying: tuleap-lister: code review: Update py files perms -X.
Applying: tuleap-lister: code review: minimise json files.
Changes applied before test
commit 882696976b89b10bd6e94445bc0fd52884914b27
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 16:33:24 2021 +0200

    tuleap-lister: code review: minimise json files.

commit 5512158dc23e1c1fd67de4fa4b10f4d6872c9714
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 16:26:47 2021 +0200

    tuleap-lister: code review: Update py files perms -X.

commit a4da01a54b615f7984f928f3bbb0f149074c05fb
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 16:25:28 2021 +0200

    tuleap-lister: code review: Update copyright to 2021 exactly.

commit 9a2874f4243e7adcecafc30ced04b0c24fc645ec
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 16:18:40 2021 +0200

    tuleap-lister: code review: Update tutorial for misc files to edit.

commit 497cad57caf69b3c3196268623c6fa24c63c3d10
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 15:46:02 2021 +0200

    tuleap-lister: code review: add name to CONTRIBUTORS file.

commit 5a7a962ea61e9f4df6619b6aac638999388b6e8b
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 15:43:07 2021 +0200

    tuleap-lister: code review: improve results_simplified for svn repos.

commit 43165ef2dd7735bc2e1f40766f41a1cd708d97d9
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 10:55:36 2021 +0200

    tuleap-lister: code review: Remove authentication useless lines + fix typos.

commit d88f3a4dfd53f366eeaa6b36ce265f7dd72a48cb
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:52:36 2021 +0200

    tuleap-lister: code review: fix test_task kwargs.

commit 35c619935cf1d7ce5b0a525b842d811a7a33dd6d
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:41:43 2021 +0200

    tuleap-lister: code review: fix relister > lister.

commit 71c2ef5551d817b066c37497ac50b097abbdcbad
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:39:24 2021 +0200

    tuleap-lister: code review: fix mocker + tests/setup_cli.

commit 1d7f0d856dfba61229135116525369b4efc3e047
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:06:54 2021 +0200

    tuleap-lister: Add rate-limiting test + fix debug and typo.

commit 59b26a8f94edc0310f0ede888436cb7006ec064a
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 15:00:32 2021 +0200

    tuleap-lister: fix args in test_task.

commit 78c1c2ec09da7bce0497dd477ed810415d61ea84
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 14:44:55 2021 +0200

    tuleap: initialise lister.

See https://jenkins.softwareheritage.org/job/DLS/job/tests-on-diff/291/ for more details.

Harbormaster completed remote builds in B21570: Diff 20605.May 20 2021, 4:39 PM
ardumont added inline comments.May 20 2021, 4:44 PM
docs/tutorial.rst
362 ↗(On Diff #20605)

great ;)

ardumont added a comment.May 20 2021, 5:07 PM

Looks good to me.

Note that i did not look into the nitty gritty details of the tuleap api though.

There are still a couple of suggestions. We tend to name with explicit and meaningful
names our variables. I've highlighted one but there are other locations where it'd be
best to fix. Also, some conditional are not covered by tests. Nonetheless, I think it's
fine. Great work, thanks.

A next step which would be nice to have would be to run this lister within a docker
environment to finalize eventual papercuts not yet covered here (maybe there ain't any,
no idea ;). The following docs [1] would definitely help.

[1] https://docs.softwareheritage.org/devel/getting-started/using-docker.html

swh/lister/tuleap/lister.py
133–137

Might as well make those variable meaningful, don't really get the initial gurl term ¯\_(ツ)_/¯ (please, rename to something clearer or righter if i'm wrong heh ;)

swh/lister/tuleap/tests/test_lister.py
29

neat ;)

96–97

No need for the indirection here.

123

drop unneeded comment.

ardumont accepted this revision as: ardumont.May 20 2021, 5:08 PM
ardumont added 1 blocking reviewer(s): Reviewers.
ardumont added a comment.May 21 2021, 11:48 AM

@borisbaldassari I forgot to mention, when someone else is done (might be @vlorentz ;)
with the review, please squash your commits into one prior to push it.

borisbaldassari added a comment.May 21 2021, 12:10 PM

@ardumont @vlorentz thanks a lot for all the help and inputs.

Please DON'T merge it yet however, as I still need to do some work on it:

  • add svn repos,
  • check rate-limits (and the need for an incremental lister),
  • control variable names and apply ardumont's suggestions,
  • run it in a docker-dev env,
  • squash all commits (thanks for the hint).

I'll do that in the upcoming days as time allows and let you know. Thanks, have a wonderful day!

ardumont added a comment.May 21 2021, 6:43 PM

@ardumont @vlorentz thanks a lot for all the help and inputs.

You are welcome.

Please DON'T merge it yet however,

Don't worry about it, we let authors merge their work. So they can enjoy the
satisfaction of a work well done ;p

as I still need to do some work on it:

  • add svn repos,
  • check rate-limits (and the need for an incremental lister),
  • control variable names and apply ardumont's suggestions,
  • run it in a docker-dev env,
  • squash all commits (thanks for the hint).

Awesome plan!

I'll do that in the upcoming days as time allows and let you know.

Great

Thanks, have a wonderful day!

Thanks. Have a nice day as well ;)

Cheers,

borisbaldassari updated this revision to Diff 20625.May 23 2021, 3:36 PM
  • tuleap-lister: code review: fix chmod on json files.
  • tuleap-lister: code review: fix var names + add tests.
  • tuleap-lister: code review: fix useless indirection.
swh-public-ci added a comment.May 23 2021, 3:39 PM

Build is green

Patch application report for D5754 (id=20625)

Rebasing onto 8f3bbacd5e...

First, rewinding head to replay your work on top of it...
Applying: tuleap: initialise lister.
Applying: tuleap-lister: fix args in test_task.
Applying: tuleap-lister: Add rate-limiting test + fix debug and typo.
Applying: tuleap-lister: code review: fix mocker + tests/setup_cli.
Applying: tuleap-lister: code review: fix relister > lister.
Applying: tuleap-lister: code review: fix test_task kwargs.
Applying: tuleap-lister: code review: Remove authentication useless lines + fix typos.
Applying: tuleap-lister: code review: improve results_simplified for svn repos.
Applying: tuleap-lister: code review: add name to CONTRIBUTORS file.
Applying: tuleap-lister: code review: Update tutorial for misc files to edit.
Applying: tuleap-lister: code review: Update copyright to 2021 exactly.
Applying: tuleap-lister: code review: Update py files perms -X.
Applying: tuleap-lister: code review: minimise json files.
Applying: tuleap-lister: code review: fix chmod on json files.
Applying: tuleap-lister: code review: fix var names + add tests.
Applying: tuleap-lister: code review: fix useless indirection.
Changes applied before test
commit a9e1a8bae756bff0a46db4ec788606d5cf3b3d16
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Sun May 23 15:35:53 2021 +0200

    tuleap-lister: code review: fix useless indirection.

commit 29250ed510440e3bed310a7ad340a4e78c1ea8b8
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Sun May 23 14:04:58 2021 +0200

    tuleap-lister: code review: fix var names + add tests.

commit f99641a171d70c76d30a67d4afc646903e84b277
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Sun May 23 13:45:53 2021 +0200

    tuleap-lister: code review: fix chmod on json files.

commit dbc8784f797ab0603df7359c18549bdf19c89059
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 16:33:24 2021 +0200

    tuleap-lister: code review: minimise json files.

commit 6cd8a2cde2017ef5ad13f744d97d0247ec59bc6e
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 16:26:47 2021 +0200

    tuleap-lister: code review: Update py files perms -X.

commit 1bf6a577f714a9a8f9c01bbf453c136f036e12ec
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 16:25:28 2021 +0200

    tuleap-lister: code review: Update copyright to 2021 exactly.

commit df51106498ee24716456678aff8f220e197b0172
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 16:18:40 2021 +0200

    tuleap-lister: code review: Update tutorial for misc files to edit.

commit 2aeac53a00756f1e6b47833de84aaee2f7ae831d
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 15:46:02 2021 +0200

    tuleap-lister: code review: add name to CONTRIBUTORS file.

commit e3485626ca34426f6fe128fef304f42edcc642d2
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 15:43:07 2021 +0200

    tuleap-lister: code review: improve results_simplified for svn repos.

commit 5dab447205bfef4ae62983094631c48a1950cebd
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 10:55:36 2021 +0200

    tuleap-lister: code review: Remove authentication useless lines + fix typos.

commit 7eacf8dd2f24c204f1801a7099d09d8d3a27250a
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:52:36 2021 +0200

    tuleap-lister: code review: fix test_task kwargs.

commit 8e3234bd749c2938281eb7c116937adf3e5d38fd
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:41:43 2021 +0200

    tuleap-lister: code review: fix relister > lister.

commit 6480aa7f2c0d842bd5cf1c5331613a0e5319a897
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:39:24 2021 +0200

    tuleap-lister: code review: fix mocker + tests/setup_cli.

commit 8dca3e1c0abb2c3159cab69eb3b1708a908541ed
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:06:54 2021 +0200

    tuleap-lister: Add rate-limiting test + fix debug and typo.

commit bcfa66320abfeace0b4600d98277682149d57cae
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 15:00:32 2021 +0200

    tuleap-lister: fix args in test_task.

commit d3bd3a443dc4faf301e3dfb8fd5422117a0d0cc4
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 14:44:55 2021 +0200

    tuleap: initialise lister.

See https://jenkins.softwareheritage.org/job/DLS/job/tests-on-diff/292/ for more details.

Harbormaster completed remote builds in B21589: Diff 20625.May 23 2021, 3:39 PM
borisbaldassari updated this revision to Diff 20679.May 26 2021, 11:06 AM
  • tuleap-lister: code review: Add empty repo test, minor typo fixes.
swh-public-ci added a comment.May 26 2021, 11:10 AM

Build is green

Patch application report for D5754 (id=20679)

Rebasing onto 8f3bbacd5e...

First, rewinding head to replay your work on top of it...
Applying: tuleap: initialise lister.
Applying: tuleap-lister: fix args in test_task.
Applying: tuleap-lister: Add rate-limiting test + fix debug and typo.
Applying: tuleap-lister: code review: fix mocker + tests/setup_cli.
Applying: tuleap-lister: code review: fix relister > lister.
Applying: tuleap-lister: code review: fix test_task kwargs.
Applying: tuleap-lister: code review: Remove authentication useless lines + fix typos.
Applying: tuleap-lister: code review: improve results_simplified for svn repos.
Applying: tuleap-lister: code review: add name to CONTRIBUTORS file.
Applying: tuleap-lister: code review: Update tutorial for misc files to edit.
Applying: tuleap-lister: code review: Update copyright to 2021 exactly.
Applying: tuleap-lister: code review: Update py files perms -X.
Applying: tuleap-lister: code review: minimise json files.
Applying: tuleap-lister: code review: fix chmod on json files.
Applying: tuleap-lister: code review: fix var names + add tests.
Applying: tuleap-lister: code review: fix useless indirection.
Applying: tuleap-lister: code review: Add empty repo test, minor typo fixes.
Changes applied before test
commit 9958ec45af948aa351776b84c8a479c16536172c
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 26 11:05:42 2021 +0200

    tuleap-lister: code review: Add empty repo test, minor typo fixes.

commit 099ff0bc7c306a5352f1ad5697f498059989c238
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Sun May 23 15:35:53 2021 +0200

    tuleap-lister: code review: fix useless indirection.

commit 0061ed2d5c3c38e6a963d6cf499035125c6daf9a
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Sun May 23 14:04:58 2021 +0200

    tuleap-lister: code review: fix var names + add tests.

commit f66463fb7e7702600661dc5c4c1227b04048e833
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Sun May 23 13:45:53 2021 +0200

    tuleap-lister: code review: fix chmod on json files.

commit 8d08e45e0bddb1912ce6e99f3e81107141c17c33
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 16:33:24 2021 +0200

    tuleap-lister: code review: minimise json files.

commit ea27eb4f1c3dedebfd52f2257d95f5bdaee6e923
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 16:26:47 2021 +0200

    tuleap-lister: code review: Update py files perms -X.

commit e07d6e60eae9126dafe59e2870d4fc91fcc6f390
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 16:25:28 2021 +0200

    tuleap-lister: code review: Update copyright to 2021 exactly.

commit 8c7854d2c55ab4832ec9d99e9305e967f2076733
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 16:18:40 2021 +0200

    tuleap-lister: code review: Update tutorial for misc files to edit.

commit d8113ea656ea994dcc5abb9eaf3a623b86f7044d
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 15:46:02 2021 +0200

    tuleap-lister: code review: add name to CONTRIBUTORS file.

commit bbeb1da283335f17fc12a51a843b53630063c72a
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 15:43:07 2021 +0200

    tuleap-lister: code review: improve results_simplified for svn repos.

commit ea29633fb85e1819db8f5e9fd0f1cfcb1b7e4f54
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Thu May 20 10:55:36 2021 +0200

    tuleap-lister: code review: Remove authentication useless lines + fix typos.

commit 296e15394e10af95b92c41a9285f66e767974df2
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:52:36 2021 +0200

    tuleap-lister: code review: fix test_task kwargs.

commit 3a7a95fe5d32561523b97fb4d76c7645a1e43510
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:41:43 2021 +0200

    tuleap-lister: code review: fix relister > lister.

commit 2228f9b38ad2dd7ce7839cc0416d4241fc9d1e9a
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:39:24 2021 +0200

    tuleap-lister: code review: fix mocker + tests/setup_cli.

commit b01cf2ccf854b09a8b3b2ac570164815a269d3ae
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 17:06:54 2021 +0200

    tuleap-lister: Add rate-limiting test + fix debug and typo.

commit 791025bc88ede3316d8f1c54bb6dc41bdcb5cba7
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 15:00:32 2021 +0200

    tuleap-lister: fix args in test_task.

commit b415d2150e3459522d449c7f86039c284478f473
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 14:44:55 2021 +0200

    tuleap: initialise lister.

See https://jenkins.softwareheritage.org/job/DLS/job/tests-on-diff/293/ for more details.

Harbormaster completed remote builds in B21643: Diff 20679.May 26 2021, 11:10 AM
borisbaldassari updated this revision to Diff 20680.May 26 2021, 11:10 AM

D5754: tuleap: initialise lister.

swh-public-ci added a comment.May 26 2021, 11:14 AM

Build is green

Patch application report for D5754 (id=20680)

Rebasing onto 8f3bbacd5e...

First, rewinding head to replay your work on top of it...
Applying: tuleap: initialise lister.
Changes applied before test
commit a96907919954fe361f1d777f309b745f0f577b96
Author: Boris Baldassari <boris@chrysalice.org>
Date:   Wed May 19 14:44:55 2021 +0200

    tuleap: initialise lister.
    
    tuleap-lister: fix args in test_task.
    
    tuleap-lister: Add rate-limiting test + fix debug and typo.
    
    tuleap-lister: code review: fix mocker + tests/setup_cli.
    
    tuleap-lister: code review: fix relister > lister.
    
    tuleap-lister: code review: fix test_task kwargs.
    
    tuleap-lister: code review: Remove authentication useless lines + fix typos.
    
    tuleap-lister: code review: improve results_simplified for svn repos.
    
    tuleap-lister: code review: add name to CONTRIBUTORS file.
    
    tuleap-lister: code review: Update tutorial for misc files to edit.
    
    tuleap-lister: code review: Update copyright to 2021 exactly.
    
    tuleap-lister: code review: Update py files perms -X.
    
    tuleap-lister: code review: minimise json files.
    
    tuleap-lister: code review: fix chmod on json files.
    
    tuleap-lister: code review: fix var names + add tests.
    
    tuleap-lister: code review: fix useless indirection.
    
    tuleap-lister: code review: Add empty repo test, minor typo fixes.

See https://jenkins.softwareheritage.org/job/DLS/job/tests-on-diff/294/ for more details.

Harbormaster completed remote builds in B21644: Diff 20680.May 26 2021, 11:14 AM
borisbaldassari added a comment.May 26 2021, 11:40 AM

I think I'm done for now. It works well on docker-dev, tests pass, all review comments have been treated.
For the records the following improvements could be implemented:

  • support for svn repos -- I can't find any tuleap forge with svn repos, so I'll drop the feature for now.
  • authentication -- not a big deal, but tuleap.net has 41 repositories so rate-limiting and auth aren't absolutely required. I'm waiting for inputs from enalean (makers of tuleap) to decide on what would actually be needed.
swh/lister/tuleap/lister.py
133–137

You're definitely right. I've renamed almost all variables to be more meaningful, and did some code cleaning.
(gurl was for git url actually ;-)

ardumont accepted this revision.May 26 2021, 12:28 PM

I think I'm done for now. It works well on docker-dev, tests pass, all review comments have been treated.

Neat, thanks.

For the records the following improvements could be implemented:

  • support for svn repos -- I can't find any tuleap forge with svn repos, so I'll drop the feature for now.

authentication -- not a big deal, but tuleap.net has 41 repositories so rate-limiting and auth aren't absolutely required. I'm waiting for inputs from enalean (makers of tuleap) to decide on what would actually be needed.

Sounds reasonable.

Cheers,

This revision is now accepted and ready to land.May 26 2021, 12:28 PM
ardumont added a comment.Jun 2 2021, 3:15 PM

By the way @borisbaldassari, you can push, that should land and close the diff ;)

borisbaldassari added a comment.Jun 5 2021, 10:04 AM

Hey @ardumont , ok, was not sure of what to do next. Thanks, just pushed it.
BTW I got some news from the nice Enalean people, so I'll add the authentication and svn repos soon, in another task/diff/push.

borisbaldassari added a comment.Jun 9 2021, 8:52 AM
In D5754#147866, @ardumont wrote:

By the way @borisbaldassari, you can push, that should land and close the diff ;)

Hum.. I pushed a couple of days ago and the diff is still open. I've probably done the wrong thing.

Changes were pushed to origin/T3334_tuleap_lister -- is that ok? should I do something else/more to automatically close the diff (and see the new lister on the master etc.)?

douardda added a subscriber: douardda.Jun 9 2021, 12:09 PM
In D5754#148570, @borisbaldassari wrote:
In D5754#147866, @ardumont wrote:

By the way @borisbaldassari, you can push, that should land and close the diff ;)

Hum.. I pushed a couple of days ago and the diff is still open. I've probably done the wrong thing.

Changes were pushed to origin/T3334_tuleap_lister -- is that ok? should I do something else/more to automatically close the diff (and see the new lister on the master etc.)?

you probably rework your git history without updating the Diff, so phab wait for the revision it knows about to be merged in master (aka 04c0a50706e8 here), but if you modified it, another revision has been pushed in the main repo, and phab is not smart enough to detect it is actually an updated version of the diff that reached the git repo.

ardumont added a comment.Jun 9 2021, 12:13 PM

Sorry, i missed the good news!

Awesome (both the push and the future diff about the evolution for svn and authentication).

David explained everything already. So you can close this diff and maybe mention the commit closing this at the same time in the comment.

Cheers,

douardda added a comment.Jun 9 2021, 12:14 PM
In D5754#148709, @douardda wrote:
In D5754#148570, @borisbaldassari wrote:
In D5754#147866, @ardumont wrote:

By the way @borisbaldassari, you can push, that should land and close the diff ;)

Hum.. I pushed a couple of days ago and the diff is still open. I've probably done the wrong thing.

Changes were pushed to origin/T3334_tuleap_lister -- is that ok? should I do something else/more to automatically close the diff (and see the new lister on the master etc.)?

you probably rework your git history without updating the Diff, so phab wait for the revision it knows about to be merged in master (aka 04c0a50706e8 here), but if you modified it, another revision has been pushed in the main repo, and phab is not smart enough to detect it is actually an updated version of the diff that reached the git repo.

well in this case, I don't see your commit in the main git repo. Are you sure you pushed it on the master branch? Humm, I see a T3334_tuleap_lister branch, so I guess not.

So: you must push your revisions on the master branch. But now some more revisions has been pushed there, you need to rebase your revision on the top of current origin/master, then update yout diff to make phabricator aware of this new version of your diff, then 'git push' your local master branch (including you revision) on origin/master.

ardumont added a comment.Jun 9 2021, 12:21 PM

David explained everything already. So you can close this diff and maybe mention the commit closing this at the same time in the comment.

oops, yes, sorry i read too fast. You pushed a remote branch.
You need to follow what david said (and now i'm stopping adding noise here).

douardda added a comment.Jun 9 2021, 12:22 PM
In D5754#148712, @ardumont wrote:

David explained everything already. So you can close this diff and maybe mention the commit closing this at the same time in the comment.

oops, yes, sorry i read too fast. You pushed a remote branch.
You need to follow what david said (and now i'm stopping adding noise here).

as discussed on irc, I'll handle the merge.

Closed by commit rDLS04c0a50706e8: tuleap: initialise lister. (authored by borisbaldassari). · Explain WhyJun 9 2021, 12:31 PM
This revision was automatically updated to reflect the committed changes.
borisbaldassari added a commit: rDLS04c0a50706e8: tuleap: initialise lister..

Revision Contents
Changeset List

PathSize
3 lines
1 line
swh/
lister/
tuleap/
12 lines
169 lines
22 lines
tests/
data/
https_tuleap.net/
170 lines
18 lines
18 lines
141 lines
54 lines

Diff 20569

View Options

README.md

Loading...
View Options

setup.py

Loading...
View Options

swh/lister/tuleap/__init__.py

Loading...
View Options

swh/lister/tuleap/lister.py

Loading...
View Options

swh/lister/tuleap/tasks.py

Loading...
View Options

swh/lister/tuleap/tests/__init__.py

Loading...
View Options

swh/lister/tuleap/tests/data/https_tuleap.net/projects

Loading...
View Options

swh/lister/tuleap/tests/data/https_tuleap.net/repo_1

Loading...
View Options

swh/lister/tuleap/tests/data/https_tuleap.net/repo_2

Loading...
View Options

swh/lister/tuleap/tests/test_lister.py

Loading...
View Options

swh/lister/tuleap/tests/test_tasks.py

Loading...
About · Developer information · Legal