Page MenuHomeSoftware Heritage
Differential D5232

Implement counters pipeline
ClosedPublic
Actions

Authored by vsellier on Mar 12 2021, 10:08 AM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T2912: Next generation archive counters
Commits
rDCNTd90ee1974638: Implement counters pipeline
Summary

It uses a pytest-redis library to test the code related to redis.This lib is not packaged in debian so we'll need to build it and push on our debian repository
Redis also needs to be added on the build environment

Related to T2912

Diff Detail

Repository
rDCNT Archive counters
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vsellier created this revision.Mar 12 2021, 10:08 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptMar 12 2021, 10:08 AM
swh-public-ci added a comment.Mar 12 2021, 10:09 AM

Build has FAILED

Patch application report for D5232 (id=18762)

Rebasing onto 826c30e4b7...

Current branch diff-target is up to date.
Changes applied before test
commit 9bfd9f991afc9ca966a7669b2836afde537fce04
Author: Vincent SELLIER <vincent.sellier@softwareheritage.org>
Date:   Thu Mar 11 14:35:50 2021 +0100

    Implement counters pipeline
    
    Related to T2912

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

Harbormaster returned this revision to the author for changes because remote builds failed.Mar 12 2021, 10:09 AM
Harbormaster failed remote builds in B19858: Diff 18762!
vsellier retitled this revision from Implement counters pipeline It uses a `pytest-redis` library to test the code related to redis. This lib is not packaged in debian so we'll need to build it and push on our debian repository to Implement counters pipeline.Mar 12 2021, 10:13 AM
vsellier edited the summary of this revision. (Show Details)
vsellier edited the summary of this revision. (Show Details)
vsellier mentioned this in T3120: Fix swh-jenkins/base-buster docker image build.Mar 12 2021, 11:08 AM
swh-public-ci added a comment.Mar 15 2021, 4:02 PM

Build has FAILED

Patch application report for D5232 (id=18762)

Rebasing onto 826c30e4b7...

Current branch diff-target is up to date.
Changes applied before test
commit 9bfd9f991afc9ca966a7669b2836afde537fce04
Author: Vincent SELLIER <vincent.sellier@softwareheritage.org>
Date:   Thu Mar 11 14:35:50 2021 +0100

    Implement counters pipeline
    
    Related to T2912

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

swh-public-ci added a comment.Mar 15 2021, 4:28 PM

Build is green

Patch application report for D5232 (id=18762)

Rebasing onto 826c30e4b7...

Current branch diff-target is up to date.
Changes applied before test
commit 9bfd9f991afc9ca966a7669b2836afde537fce04
Author: Vincent SELLIER <vincent.sellier@softwareheritage.org>
Date:   Thu Mar 11 14:35:50 2021 +0100

    Implement counters pipeline
    
    Related to T2912

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

Harbormaster completed remote builds in B19858: Diff 18762.Mar 15 2021, 4:28 PM
vsellier requested review of this revision.Mar 15 2021, 4:28 PM
ardumont added a subscriber: ardumont.Mar 15 2021, 4:37 PM

great

couple of comments inline.

swh/counters/redis.py
32–37

You can use an @property attribute like ^.
And then refer to self.redis_client (except for the self._redis_client = None under class up above) everywhere.
Then that code will get executed transparently.

(suggestion ^)

swh/counters/tests/test_journal_client.py
24

you can clean up a bit :)

swh/counters/tests/test_redis.py
27

Isn't redise a typo and redundant a bit?

ardumont accepted this revision.Mar 15 2021, 5:14 PM
This revision is now accepted and ready to land.Mar 15 2021, 5:14 PM
vsellier added inline comments.Mar 15 2021, 5:51 PM
swh/counters/redis.py
32–37

Nice, I will use this

swh/counters/tests/test_journal_client.py
24

sure :)

vsellier updated this revision to Diff 18834.Mar 15 2021, 6:51 PM

review feedbacks:

  • rename the strangly named test
  • use a getter for the redis client property
  • cleanup code
swh-public-ci added a comment.Mar 15 2021, 6:52 PM

Build is green

Patch application report for D5232 (id=18834)

Rebasing onto 826c30e4b7...

Current branch diff-target is up to date.
Changes applied before test
commit d90ee1974638bed7e1f2afdf9c5a4ea0233c0594
Author: Vincent SELLIER <vincent.sellier@softwareheritage.org>
Date:   Thu Mar 11 14:35:50 2021 +0100

    Implement counters pipeline
    
    Related to T2912

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

Harbormaster completed remote builds in B19927: Diff 18834.Mar 15 2021, 6:52 PM
ardumont accepted this revision.Mar 15 2021, 6:59 PM

lgtm

Closed by commit rDCNTd90ee1974638: Implement counters pipeline (authored by vsellier). · Explain WhyMar 15 2021, 8:03 PM
This revision was automatically updated to reflect the committed changes.
vsellier added a commit: rDCNTd90ee1974638: Implement counters pipeline.

Revision Contents
Changeset List

PathSize
3 lines
1 line
1 line
swh/
counters/
10 lines
45 lines
tests/
3 lines
28 lines
68 lines

Diff 18835

View Options

mypy.ini

Loading...
View Options

requirements-test.txt

Loading...
View Options

requirements.txt

Loading...
View Options

swh/counters/journal_client.py

Loading...
View Options

swh/counters/redis.py

Loading...
View Options

swh/counters/tests/test_cli.py

Loading...
View Options

swh/counters/tests/test_journal_client.py

Loading...
View Options

swh/counters/tests/test_redis.py

Loading...
About · Developer information · Legal