Page MenuHomePhabricator

Decouple Quibble planning and execution phases
Closed, ResolvedPublic

Description

This is more of a question than a suggestion--it occurred to me that we might want to encapsulate each step in Quibble as an instance of a Command object, so it becomes possible to create the execution plan before actually executing anything. Execution would happen in a second phase, in which we call each command sequentially.

There are several benefits to this approach. A few thoughts:

  • Decoupling planning and execution code should make it easier to streamline and reuse components.
  • More sophisticated and performant execution models become possible in the future, such as concurrency and dependency trees.
  • We can run in a "no action" mode where the steps are printed but nothing is executed (other than cloning...)

One complication is that some steps cannot be fully planned until dependencies are resolved, for example we don't know whether selenium tests should be run until the repository is cloned. As a quick and dirty rough pass, we could split the job into high-level workflows which each have their own plan and execute phases, i.e. * clone just the ZUUL_PROJECT, plan and execute linting, then * clone remaining repos, plan and execute integrating tests.

Related Objects

Mentioned In
T220199: Quibble: deprecate and phase out EXT_DEPENDENCIES / SKIN_DEPENDENCIES
T218357: Quibble space separated options shallow arguments
T211701: Quibble should clone repositories in parallel
rQUIBBLE6d281cd87c07: Separate planning and execution phases
rQUIBBLE406fd2f1ad00: browser tests as command
rQUIBBLE3a60fb7775e4: Core npm composer test as command
rQUIBBLEb98853ccf33e: Separate planning and execution phases
rQUIBBLE7e1442de3b0a: browser tests as command
rQUIBBLE9d82f17dfab1: Separate planning and execution phases
rQUIBBLEc28f33bf9b04: browser tests as command
rQUIBBLE63225b806f16: Separate planning and execution phases
rQUIBBLE45219e3c418c: browser tests as command
rQUIBBLEb2fb7a1d703b: Separate planning and execution phases
rQUIBBLEf7c51a8b0bab: browser tests as command
rQUIBBLE7413e3e0aa14: Core npm composer test as command
rQUIBBLEbcc695fe1e1e: phpunit tests as command
rQUIBBLEb5d689ec027e: Composer and npm test as command
rQUIBBLEa663ad60cdff: MediaWiki installation... as command
rQUIBBLEf0ff523faede: MediaWiki installation... as command
rQUIBBLE3c3d43108f33: Npm install as command
rQUIBBLE27439695b871: Composer and npm test as command
rQUIBBLE64f630119302: Vendor dependencies as command
rQUIBBLE38c99216c68e: Composer dependencies as command
rQUIBBLE6efafaa941a5: Extension and skin submodule update as command
rQUIBBLE4e2bad4a7a9c: Extract zuul clone into a command object
rQUIBBLE732e6328c4fd: Separate planning and execution phases
rQUIBBLEa0e58b4d6c6a: MediaWiki installation... as command
rQUIBBLEcef071a9f8f5: browser tests as command
rQUIBBLE9561b5455818: Core npm composer test as command
rQUIBBLEc878e978952b: phpunit tests as command
rQUIBBLE802fd225e82c: Composer and npm test as command
rQUIBBLE5d7d8c541c11: Npm install as command
rQUIBBLE838ae264cb4f: Vendor dependencies as command
rQUIBBLE8d09ad8a98a3: Composer dependencies as command
rQUIBBLEfb0794253df2: Extension and skin submodule update as command
rQUIBBLE987ea9107936: Extract zuul clone into a command object
T224969: 20% time things for 5/29
rQUIBBLE8f189f111908: Extension and skin submodule update as command
rQUIBBLE0441b5df9597: Extension and skin submodule update as command
rQUIBBLE0cae0b60d64f: Composer dependencies as command
rQUIBBLE127fdac4862d: Separate planning and execution phases
rQUIBBLE5065b63a1323: MediaWiki installation... as command
rQUIBBLE8ff34534d4f2: browser tests as command
rQUIBBLE50fc12b96651: Core npm composer test as command
rQUIBBLE48e8b2d087de: phpunit tests as command
rQUIBBLEcf798490efda: Npm install as command
rQUIBBLE35d9f3eeae83: Composer and npm test as command
rQUIBBLEf334a0da62c8: Vendor dependencies as command
rQUIBBLE03e9c15eb02f: Composer dependencies as command
rQUIBBLE43dc9f774e1e: Extension and skin submodule update as command
rQUIBBLE5c45699a76e6: Extract zuul clone into a command object
T223967: Tangential CI work to support Tech Wishes
rQUIBBLE8e04b546bd50: Separate planning and execution phases
rQUIBBLEbeeb62a3bd58: Separate planning and execution phases
rQUIBBLEc18e6e8cb42f: [WIP] Rough out plan-execution decoupling
rQUIBBLEe6a8d8dc426e: [WIP] Rough out plan-execution decoupling
rQUIBBLEa64887754e33: [WIP] Rough out plan-execution decoupling
rQUIBBLEf565174b7099: [WIP] Rough out plan-execution decoupling
rQUIBBLE882c23e250ce: [WIP] Rough out plan-execution decoupling
Mentioned Here
T211701: Quibble should clone repositories in parallel
T218357: Quibble space separated options shallow arguments
T220199: Quibble: deprecate and phase out EXT_DEPENDENCIES / SKIN_DEPENDENCIES
rQUIBBLEc0fe6ebdc0b7: Move parallel_run to util.py

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

I ran into another complication: the zuul cloner takes a huge number of parameters, plus reading from the environment. The command abstraction shouldn't explicitly wire the parameters together because that would be fragile and high-maintenance. The only trick I've come up with so far is to separate the parameters into a list of projects, then an opaque **kwargs which is built by the calling code and by the quibble.zuul module. I don't like it, but there is one small benefit, that it facilitates dumping the entire list of parameters to zuul for debugging.

Change 511749 had a related patch set uploaded (by Awight; owner: Awight):
[integration/quibble@master] [WIP] Rough out plan-execution decoupling

https://gerrit.wikimedia.org/r/511749

Some successes in the WIP patch so far. It actually runs through the whole process through browser testing, at least when smoke-testing a trivial code path. It prints a medium-granularity execution plan, and flag --plan-only stops before execution. I'm happy about how all command parameters must be explicitly passed into the constructor, so there are no self.* surprises which might cause implicit coupling issues later, and the command implementations can be extracted into separate modules as desired. Personally, I think that's a good direction to go in, so that each module has one general responsibility. The ZuulCloneCommand build_params hack isn't completely disgusting, which is unexpected.

I'd like to break up the workflows into slightly more granular steps, and find a way to analyze conditions like "is there a tests/selenium directory in this repo?" immediately after cloning, using that information to expand high-level steps, but I don't have a good picture of how to do that yet. The main plan generator built up by yield statements in build_execution_plan should be replaced by some other type of explicit accumulator, e.g. self.add_step.

awight moved this task from Backlog to In progress on the Quibble board.

Change 513377 had a related patch set uploaded (by Awight; owner: Awight):
[integration/quibble@master] Extract zuul clone into a command object

https://gerrit.wikimedia.org/r/513377

Change 513406 had a related patch set uploaded (by Awight; owner: Awight):
[integration/quibble@master] Extension and skin submodule update as command

https://gerrit.wikimedia.org/r/513406

Change 513498 had a related patch set uploaded (by Awight; owner: Awight):
[integration/quibble@master] Composer dependencies as command

https://gerrit.wikimedia.org/r/513498

Change 513503 had a related patch set uploaded (by Awight; owner: Awight):
[integration/quibble@master] Vendor dependencies as command

https://gerrit.wikimedia.org/r/513503

Change 513504 had a related patch set uploaded (by Awight; owner: Awight):
[integration/quibble@master] Composer and npm test as command

https://gerrit.wikimedia.org/r/513504

Change 513510 had a related patch set uploaded (by Awight; owner: Awight):
[integration/quibble@master] Npm install as command

https://gerrit.wikimedia.org/r/513510

Change 513513 had a related patch set uploaded (by Awight; owner: Awight):
[integration/quibble@master] phpunit tests as command

https://gerrit.wikimedia.org/r/513513

Change 513515 had a related patch set uploaded (by Awight; owner: Awight):
[integration/quibble@master] Core npm composer test as command

https://gerrit.wikimedia.org/r/513515

Change 513518 had a related patch set uploaded (by Awight; owner: Awight):
[integration/quibble@master] browser tests as command

https://gerrit.wikimedia.org/r/513518

Change 513520 had a related patch set uploaded (by Awight; owner: Awight):
[integration/quibble@master] MediaWiki installation... as command

https://gerrit.wikimedia.org/r/513520

hashar assigned this task to awight.Jun 6 2019, 9:53 AM
hashar added a subscriber: hashar.

Decoupling planning and execution code should make it easier to streamline and reuse components.
We can run in a "no action" mode where the steps are printed but nothing is executed (other than cloning...)

@awight patches above implement those two. It is nice, it is taking me a bit of time to get them reviewed but I really really like the approach.

More sophisticated and performant execution models become possible in the future, such as concurrency and dependency trees.

Definitely! That indeed opens a bright new future. Previously I did a bit of concurrent execution but both were implemented differently and not very nice. I also once thought about building a dependency tree but it went nowhere :-\

Change 513377 merged by jenkins-bot:
[integration/quibble@master] Extract zuul clone into a command object

https://gerrit.wikimedia.org/r/513377

Change 513406 merged by jenkins-bot:
[integration/quibble@master] Extension and skin submodule update as command

https://gerrit.wikimedia.org/r/513406

Change 513498 merged by jenkins-bot:
[integration/quibble@master] Composer dependencies as command

https://gerrit.wikimedia.org/r/513498

Change 513503 merged by jenkins-bot:
[integration/quibble@master] Vendor dependencies as command

https://gerrit.wikimedia.org/r/513503

Change 513510 merged by jenkins-bot:
[integration/quibble@master] Npm install as command

https://gerrit.wikimedia.org/r/513510

Change 513520 merged by jenkins-bot:
[integration/quibble@master] MediaWiki installation... as command

https://gerrit.wikimedia.org/r/513520

Change 513504 merged by jenkins-bot:
[integration/quibble@master] Composer and npm test as command

https://gerrit.wikimedia.org/r/513504

Change 513513 merged by jenkins-bot:
[integration/quibble@master] phpunit tests as command

https://gerrit.wikimedia.org/r/513513

Change 513515 merged by jenkins-bot:
[integration/quibble@master] Core npm composer test as command

https://gerrit.wikimedia.org/r/513515

Change 513518 merged by jenkins-bot:
[integration/quibble@master] Browser tests as command

https://gerrit.wikimedia.org/r/513518

Change 511749 merged by jenkins-bot:
[integration/quibble@master] Separate planning and execution phases

https://gerrit.wikimedia.org/r/511749

Done by @awight after a long serie of patches that took me a while to review. One can then dump the execution plan with --dry-run and it is emitted as debug log:

$ quibble --dry-run
DEBUG:quibble.cmd:Running stages: phpunit, npm-test, composer-test, qunit, selenium
WARNING:quibble.cmd:ZUUL_PROJECT not set. Assuming mediawiki/core
INFO:quibble.cmd:Adding mediawiki/vendor
INFO:quibble.cmd:Projects: mediawiki/core, mediawiki/skins/Vector, mediawiki/vendor

DEBUG:quibble.cmd:Execution plan:
DEBUG:quibble.cmd:Zuul clone with parameters {"workers": 4, "cache_dir": "ref", "workspace": "/home/hashar/workspace/src", "projects": ["mediawiki/core", "mediawiki/skins/Vector", "mediawiki/vendor"]}
DEBUG:quibble.cmd:Extension and skin submodule update under MediaWiki root /home/hashar/workspace/src
DEBUG:quibble.cmd:Install MediaWiki, db=mysql db_dir=None vendor=True
DEBUG:quibble.cmd:Install composer dev-requires for vendor.git
DEBUG:quibble.cmd:npm install in /home/hashar/workspace/src
DEBUG:quibble.cmd:PHPUnit default suite (without database)
DEBUG:quibble.cmd:Run tests in mediawiki/core: composer, npm
DEBUG:quibble.cmd:Browser tests in /home/hashar/workspace/src: qunit, selenium (maybe) using DISPLAY=:0
DEBUG:quibble.cmd:PHPUnit default suite (with database)

Will be in the next Quibble release.

hashar closed this task as Resolved.Jun 24 2019, 1:31 PM

And released.