Page MenuHomeSoftware Heritage
Differential D5535

tests: Complete checks on message with priority consumption
ClosedPublic
Actions

Authored by ardumont on Apr 15 2021, 1:26 PM.

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Maniphest Tasks
T3084: Fast track save code now requests
Commits
rDSCH974c0c2e0512: tests: Complete checks on message with priority consumption
Summary

This completes the remaining todo from D5520.

Note that i tried to extract the following into a fixture factory [1] but somehow that
hangs. So my stance here is to let this as is as i've spent way too much time on trying
to debugging this test issue.

Related to T3084

[1] P1006

Test Plan

tox

Diff Detail

Repository
rDSCH Scheduling utilities
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.Apr 15 2021, 1:26 PM
ardumont updated this revision to Diff 19758.Apr 15 2021, 1:29 PM

Drop the todo since it's the subject of this diff

swh-public-ci added a comment.Apr 15 2021, 1:29 PM

Build is green

Patch application report for D5535 (id=19757)

Could not rebase; Attempt merge onto bfc1a87bff...

Updating bfc1a87..3ef915e
Fast-forward
 swh/scheduler/celery_backend/runner.py   |  67 +++++++++----
 swh/scheduler/tests/tasks.py             |  18 ++--
 swh/scheduler/tests/test_celery_tasks.py | 164 +++++++++++++++++++++++++++----
 swh/scheduler/tests/test_config.py       |  18 ++++
 4 files changed, 223 insertions(+), 44 deletions(-)
 create mode 100644 swh/scheduler/tests/test_config.py
Changes applied before test
commit 3ef915ede39cb18f7fedba23efb06b9b1eaff840
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 15 13:22:11 2021 +0200

    tests: Complete checks on message with priority consumption
    
    Related to T3084

commit 17052c4cfa39f7e53e4f7392586b9cd3bf84cba3
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 14 12:49:47 2021 +0200

    Route priority tasks to dedicated save code now queues
    
    This splits the calls to read tasks into 2 calls, one for tasks with no
    priority (standard), another call for tasks with priority. If any tasks with priority
    are detected, they are routed to dedicated `save_code_now:` prefixed named queues (per
    task type).
    
    Related to T3084

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

Harbormaster completed remote builds in B20775: Diff 19757.Apr 15 2021, 1:29 PM
ardumont edited the summary of this revision. (Show Details)Apr 15 2021, 1:30 PM
swh-public-ci added a comment.Apr 15 2021, 1:31 PM

Build is green

Patch application report for D5535 (id=19758)

Could not rebase; Attempt merge onto bfc1a87bff...

Updating bfc1a87..fb728f8
Fast-forward
 swh/scheduler/celery_backend/runner.py   |  67 +++++++++----
 swh/scheduler/tests/tasks.py             |  18 ++--
 swh/scheduler/tests/test_celery_tasks.py | 163 +++++++++++++++++++++++++++----
 swh/scheduler/tests/test_config.py       |  18 ++++
 4 files changed, 222 insertions(+), 44 deletions(-)
 create mode 100644 swh/scheduler/tests/test_config.py
Changes applied before test
commit fb728f80f2147ddd897686c90c619eb94db0f064
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 15 13:22:11 2021 +0200

    tests: Complete checks on message with priority consumption
    
    Related to T3084

commit 17052c4cfa39f7e53e4f7392586b9cd3bf84cba3
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 14 12:49:47 2021 +0200

    Route priority tasks to dedicated save code now queues
    
    This splits the calls to read tasks into 2 calls, one for tasks with no
    priority (standard), another call for tasks with priority. If any tasks with priority
    are detected, they are routed to dedicated `save_code_now:` prefixed named queues (per
    task type).
    
    Related to T3084

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

Harbormaster completed remote builds in B20776: Diff 19758.Apr 15 2021, 1:31 PM
ardumont requested review of this revision.Apr 15 2021, 1:31 PM
vlorentz added a subscriber: vlorentz.Apr 15 2021, 2:20 PM

lgtm, but I don't understand what this does 🤷

swh/scheduler/tests/test_celery_tasks.py
146
anlambert requested changes to this revision.Apr 15 2021, 2:21 PM
anlambert added a subscriber: anlambert.
anlambert added inline comments.
swh/scheduler/tests/test_celery_tasks.py
38–61

There is a more elegant way to add the queues to the base fixture without having to duplicate it:

@pytest.fixture(scope="session")
def swh_scheduler_celery_app(swh_scheduler_celery_app):
    swh_scheduler_celery_app.add_defaults(
        {
            "task_queues": [
                Queue(queue, Exchange(queue), routing_key=queue)
                for queue in TEST_QUEUES
            ],
        }
    )
    return swh_scheduler_celery_app
This revision now requires changes to proceed.Apr 15 2021, 2:21 PM
ardumont added a comment.EditedApr 15 2021, 2:49 PM

lgtm, but I don't understand what this does 🤷

This ensures the message with any priorities are indeed sent to the queue, the new test
only checked the tasks were marked as scheduled in the scheduler backend. And nothing
more as i could not find the proper way to configure the celery app for the highn
priority queue to be consumed.

up until this... which i'll adapt with the great suggestion from @anlambert

[1]
https://forge.softwareheritage.org/source/swh-scheduler/browse/master/swh/scheduler/celery_backend/runner.py$143-144

ardumont updated this revision to Diff 19765.Apr 15 2021, 2:55 PM

Adapt according to suggestion

swh-public-ci added a comment.Apr 15 2021, 2:57 PM

Build is green

Patch application report for D5535 (id=19765)

Rebasing onto 17052c4cfa...

Current branch diff-target is up to date.
Changes applied before test
commit 01fdd5528d70bf69f1a86cb6cc97caab217a24ac
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 15 13:22:11 2021 +0200

    tests: Complete checks on message with priority consumption
    
    Related to T3084

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

ardumont updated this revision to Diff 19766.Apr 15 2021, 2:57 PM

Fix sentence typos

Harbormaster completed remote builds in B20781: Diff 19765.Apr 15 2021, 2:57 PM
ardumont added a comment.Apr 15 2021, 2:58 PM

Thanks for the typos ping ;)
fixed.

swh-public-ci added a comment.Apr 15 2021, 3:00 PM

Build is green

Patch application report for D5535 (id=19766)

Rebasing onto 17052c4cfa...

Current branch diff-target is up to date.
Changes applied before test
commit 974c0c2e05124fea7947632f8407fb24502a345e
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 15 13:22:11 2021 +0200

    tests: Complete checks on message with priority consumption
    
    Related to T3084

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

Harbormaster completed remote builds in B20782: Diff 19766.Apr 15 2021, 3:00 PM
anlambert accepted this revision.Apr 15 2021, 3:00 PM

Looks good to me.

This revision is now accepted and ready to land.Apr 15 2021, 3:00 PM
Closed by commit rDSCH974c0c2e0512: tests: Complete checks on message with priority consumption (authored by ardumont). · Explain WhyApr 15 2021, 3:04 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDSCH974c0c2e0512: tests: Complete checks on message with priority consumption.

Revision Contents
Changeset List

PathSize
swh/
scheduler/
tests/
18 lines
71 lines

Diff 19768

View Options

swh/scheduler/tests/tasks.py

Loading...
View Options

swh/scheduler/tests/test_celery_tasks.py

Loading...
About · Developer information · Legal