Page MenuHomeSoftware Heritage
Differential D6876

Allow to specify the visit grab parameters per visit type and policy
ClosedPublic
Actions

Authored by vsellier on Jan 5 2022, 9:19 AM.

Details

Reviewers
ardumont
olasd
Group Reviewers
Reviewers
Maniphest Tasks
T3827: Analyze low performance scheduling
Commits
rDSCH5c836d64a5fc: Allow to specify the visit grab parameters per visit type and policy
Summary

Related to T3827

Diff Detail

Repository
rDSCH Scheduling utilities
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 25804
Build 40325: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 40324: arc lint + arc unit

Unit TestsFailed
View All

TimeTest
35 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.scheduler.tests.test_recurrent_visits::test_recurrent_visit_additional_parameters[git-tablesamples1]
swh_scheduler = <swh.scheduler.backend.SchedulerBackend object at 0x7f965e057be0> mocker = <pytest_mock.plugin.MockerFixture object at 0x7f965e4366d8> visit_type = 'git'
32 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.scheduler.tests.test_recurrent_visits::test_recurrent_visit_additional_parameters[hg-tablesamples0]
swh_scheduler = <swh.scheduler.backend.SchedulerBackend object at 0x7f965e4363c8> mocker = <pytest_mock.plugin.MockerFixture object at 0x7f965e4364e0> visit_type = 'hg', tablesamples = {}
7 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.scheduler.cli.task::swh.scheduler.cli.task.pretty_print_task
4 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.scheduler.cli.utils::swh.scheduler.cli.utils.parse_options
32 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.scheduler.tests.test_api_client::test_root
View Full Test Results (2 Failed · 332 Passed · 1 Skipped)

Event Timeline

vsellier created this revision.Jan 5 2022, 9:19 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptJan 5 2022, 9:19 AM
vsellier updated this revision to Diff 24934.Jan 5 2022, 9:21 AM

Update a remaining test value

swh-public-ci added a comment.Jan 5 2022, 9:22 AM

Build has FAILED

Patch application report for D6876 (id=24933)

Rebasing onto 559f3451c6...

Current branch diff-target is up to date.
Changes applied before test
commit 604f74c2677aedd16f1e59c5970a6558f8df1f4a
Author: Vincent SELLIER <vincent.sellier@softwareheritage.org>
Date:   Wed Jan 5 00:08:50 2022 +0100

    Allow to specify the visit grab parameters per visit type and policy
    
    Related to T3827

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

Harbormaster failed remote builds in B25804: Diff 24933!Jan 5 2022, 9:22 AM
swh-public-ci added a comment.Jan 5 2022, 9:24 AM

Build has FAILED

Patch application report for D6876 (id=24934)

Rebasing onto 559f3451c6...

Current branch diff-target is up to date.
Changes applied before test
commit 801d7a71da14994f9aee0d2d17b8d0baec1e31b8
Author: Vincent SELLIER <vincent.sellier@softwareheritage.org>
Date:   Wed Jan 5 00:08:50 2022 +0100

    Allow to specify the visit grab parameters per visit type and policy
    
    Related to T3827

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

Harbormaster returned this revision to the author for changes because remote builds failed.Jan 5 2022, 9:24 AM
Harbormaster failed remote builds in B25805: Diff 24934!
ardumont added a subscriber: ardumont.Jan 5 2022, 9:35 AM
ardumont added inline comments.
swh/scheduler/celery_backend/recurrent_visits.py
115

This one exists so might as well access it directly.
(Might even make the following assertion useless?)

ardumont added a comment.Jan 5 2022, 9:54 AM

lgtm

remains to unstuck the build.
one suggestion inline as well.

vsellier updated this revision to Diff 24935.Jan 5 2022, 10:06 AM

adapt according the feedback.
good catch, the assertion can be removed.

swh-public-ci added a comment.Jan 5 2022, 10:08 AM

Build has FAILED

Patch application report for D6876 (id=24935)

Rebasing onto 559f3451c6...

Current branch diff-target is up to date.
Changes applied before test
commit 41cf0d40aeaba790520ebf7b97165ecfc80b2682
Author: Vincent SELLIER <vincent.sellier@softwareheritage.org>
Date:   Wed Jan 5 00:08:50 2022 +0100

    Allow to specify the visit grab parameters per visit type and policy
    
    Related to T3827

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

Harbormaster returned this revision to the author for changes because remote builds failed.Jan 5 2022, 10:08 AM
Harbormaster failed remote builds in B25806: Diff 24935!
vsellier updated this revision to Diff 24936.Jan 5 2022, 11:21 AM

fix the tests for python 3.7

swh-public-ci added a comment.Jan 5 2022, 11:24 AM

Build is green

Patch application report for D6876 (id=24936)

Rebasing onto 559f3451c6...

Current branch diff-target is up to date.
Changes applied before test
commit 1b6a69a783ad45bdaca6fe42408127b74ed23720
Author: Vincent SELLIER <vincent.sellier@softwareheritage.org>
Date:   Wed Jan 5 00:08:50 2022 +0100

    Allow to specify the visit grab parameters per visit type and policy
    
    Related to T3827

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

Harbormaster completed remote builds in B25807: Diff 24936.Jan 5 2022, 11:24 AM
vsellier requested review of this revision.Jan 5 2022, 11:24 AM
ardumont accepted this revision.Jan 5 2022, 11:25 AM
This revision is now accepted and ready to land.Jan 5 2022, 11:25 AM
olasd added a subscriber: olasd.Jan 5 2022, 5:00 PM

The shape of POLICY_ADDITIONAL_PARAMETERS is a bit clunky, with the dict key being POLICY_ADDITIONAL_PARAMETERS[visit_type][kwarg][policy]; having the nested dict "key" be POLICY_ADDITIONAL_PARAMETERS[visit_type][policy] = kwargs would be simpler?

If you make

POLICY_ADDITIONAL_PARAMETERS = {
    "git": {
        "already_visited_order_by_lag": {"tablesample": 0.1},
        ...
    }
}

Then grab_next_visits can just use

grab_next_visits(
    ...,
    **POLICY_ADDITIONAL_PARAMETERS.get(visit_type, {}).get(policy, {}),
)
vsellier updated this revision to Diff 24956.Jan 5 2022, 6:21 PM

upgrade accordingly the olasd's feedback

Thanks it's simpler this way.

swh-public-ci added a comment.Jan 5 2022, 6:23 PM

Build is green

Patch application report for D6876 (id=24956)

Rebasing onto 559f3451c6...

Current branch diff-target is up to date.
Changes applied before test
commit 5c836d64a5fc460df5437e1167bf3b10613ba28b
Author: Vincent SELLIER <vincent.sellier@softwareheritage.org>
Date:   Wed Jan 5 00:08:50 2022 +0100

    Allow to specify the visit grab parameters per visit type and policy
    
    Related to T3827

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

Harbormaster completed remote builds in B25827: Diff 24956.Jan 5 2022, 6:23 PM
vsellier requested review of this revision.Jan 5 2022, 6:31 PM
olasd accepted this revision.Jan 5 2022, 11:07 PM

Awesome!

This revision is now accepted and ready to land.Jan 5 2022, 11:07 PM
Closed by commit rDSCH5c836d64a5fc: Allow to specify the visit grab parameters per visit type and policy (authored by vsellier). · Explain WhyJan 6 2022, 9:26 AM
This revision was automatically updated to reflect the committed changes.
vsellier added a commit: rDSCH5c836d64a5fc: Allow to specify the visit grab parameters per visit type and policy.

Revision Contents
Changeset List

PathSize
swh/
scheduler/
celery_backend/
22 lines
tests/
19 lines

Diff 24933

View Options

swh/scheduler/celery_backend/recurrent_visits.py

Loading...
View Options

swh/scheduler/tests/test_recurrent_visits.py

Loading...
About · Developer information · Legal