New issue
Advanced search Search tips

Issue 912154 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Swarming: cleanup cron and taskqueue URLs

Project Member Reported by mar...@chromium.org, Dec 5

Issue description

It's kind of a mess right now.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 5

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/83f91a85bd66d76225288192bea7e7ea90624c5d

commit 83f91a85bd66d76225288192bea7e7ea90624c5d
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Wed Dec 05 22:53:42 2018

[swarming] Mark many functions in lease_management as private

It's a very deep rabbit hole but the end goal is to refactor
handlers_backend.py, even if it doesn't look like it.

No functional change.

Bug: 912154
Change-Id: I7a43b43a8adc1e47b49e63428e678653d4198425
Reviewed-on: https://chromium-review.googlesource.com/c/1363538
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/83f91a85bd66d76225288192bea7e7ea90624c5d/appengine/swarming/server/lease_management.py
[modify] https://crrev.com/83f91a85bd66d76225288192bea7e7ea90624c5d/appengine/swarming/server/lease_management_test.py

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 6

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/f4c51c79fdc2e8751817c746ab18a4ada5356800

commit f4c51c79fdc2e8751817c746ab18a4ada5356800
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Thu Dec 06 17:18:00 2018

[swarming] Further refactor lease_management.py

This is actually in preparation to cleanup handlers_backend.py, but the
rabbit hole went deep.

- Mark more functions not used outside the file as private.
- Add cron_ or task_ prefix to functions as relevant and tidy
  handlers_backend.py a bit.

I struggled *a lot* making this CL because if I add 'from server import config'
inside lease_management.py, there's a unit test that fails. I haven't searched
as to why, I've made handlers_backend pass the necessary value to
cron_sync_config() instead.

No functional change.

Bug: 912154
Change-Id: I931cc52b4430ad32b9a311ca30f3ad8151d7925d
Reviewed-on: https://chromium-review.googlesource.com/c/1363536
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/f4c51c79fdc2e8751817c746ab18a4ada5356800/appengine/swarming/handlers_backend.py
[modify] https://crrev.com/f4c51c79fdc2e8751817c746ab18a4ada5356800/appengine/swarming/server/lease_management.py
[modify] https://crrev.com/f4c51c79fdc2e8751817c746ab18a4ada5356800/appengine/swarming/server/lease_management_test.py

Project Member

Comment 3 by bugdroid1@chromium.org, Dec 6

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/64d914219a1b9a15fae111c512393dabc4b15170

commit 64d914219a1b9a15fae111c512393dabc4b15170
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Thu Dec 06 18:03:52 2018

[swarming] group public code together in lease_management.py

That's my last CL to refactor this file.

No code modification, it's only move around untouched.

Bug: 912154
Change-Id: I99431bea9c19046fa74f6009c9b47fb8bf537f57
Reviewed-on: https://chromium-review.googlesource.com/c/1363542
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/64d914219a1b9a15fae111c512393dabc4b15170/appengine/swarming/server/lease_management.py

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 6

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/aa67caef5f036b237764c633be316cc97940d2ba

commit aa67caef5f036b237764c633be316cc97940d2ba
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Thu Dec 06 19:26:06 2018

[swarming] Cleanup handlers_backend.py

- Move logic from cron jobs out of handlers_backend into the relevant files as a
  standalone function.
- Create a base class _CronHandlerBase to reduce repetition.
- Remove the 'Success' output, it's useless.

Bug: 912154
Change-Id: I07cb6dc2678df4fceb8f483b92101434543e1bff
Reviewed-on: https://chromium-review.googlesource.com/c/1363532
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>

[modify] https://crrev.com/aa67caef5f036b237764c633be316cc97940d2ba/appengine/swarming/handlers_backend.py
[modify] https://crrev.com/aa67caef5f036b237764c633be316cc97940d2ba/appengine/swarming/server/bot_management.py
[modify] https://crrev.com/aa67caef5f036b237764c633be316cc97940d2ba/appengine/swarming/server/bot_management_test.py
[modify] https://crrev.com/aa67caef5f036b237764c633be316cc97940d2ba/appengine/swarming/server/task_result.py
[modify] https://crrev.com/aa67caef5f036b237764c633be316cc97940d2ba/appengine/swarming/server/task_result_test.py
[modify] https://crrev.com/aa67caef5f036b237764c633be316cc97940d2ba/appengine/swarming/server/task_scheduler.py
[modify] https://crrev.com/aa67caef5f036b237764c633be316cc97940d2ba/appengine/swarming/server/task_scheduler_test.py

Sign in to add a comment