Consolidate build workflow

[MattiSG]

RFC: Consolidate build workflow

Context

There are currently four sources of truth for how to build, test and deploy an OpenFisca package:

1. CI configuration (.circleci/config.yml, soon .github/workflows/config.yml, cf. Switch from CircleCI to GitHub Actions #1030).
2. Shell scripts run in CI but that can also be run locally (.circleci/*.sh, soon .github/*.sh).
3. Makefile.
4. OpenFisca CLI.

Depending on each case, these complement (installing locally, testing both locally and in CI, packaging in CI), reference (publish-git-tag.sh from CI config, openfisca test in test-api.sh) or overlap (make check-style vs .circleci/lint-changed-python-tests.sh) each other.

Problem

This setup raises the following issues:

1. Inconsistent workflows (make install differs from the install result in CI, which calls make build instead), leading to inconsistent test results (e.g. [4/4] [test-built] Test against packaged version #1037).
2. Inconsistent contributor practices (with maintainers more often using make test and contributors openfisca test), leading to blocked PRs (e.g. Allow variable neutralization in YAML test files #1021 (comment)).
3. Duplication of work (e.g. Retire le doublon pytest openfisca-france#1649).
4. Maintainability: where to update the workflow when it changes? This uncertainty leads to concurrent, potentially incompatible pull requests (e.g. Switch from CircleCI to GitHub Actions #1030, Run tests in a parallel matrix #1031, [4/4] [test-built] Test against packaged version #1037…).
5. Portability: shell scripts and Makefile have limited portability, and CI configuration is vendor-dependent and most often executable only on CI.

Discussion

A synchronous discussion with @benjello @benoit-cty @sandcha @maukoquiroga @HAEKADI @MattiSG took place on 02/09/2021 on this topic.

Conclusions on that topic were that:

1. There is interest in converging the workflow on one single tool.
2. Maintainers present in the call preferred make over openfisca as it is shorter, especially when multiple country packages are installed in one virtualenv (make test vs openfisca test --country-package openfisca_france tests).
3. The cost of installing Docker was felt as too high, even for maintainers.
4. The representativity of the sample of participants to the discussion was too low to lead to any conclusion as to which tool to prefer.

Annex

Known workflow steps

The following table aims at summarizing all known declared workflow steps. It was established by @MattiSG on 03/09/2021, based on reading through Core, France and Country Template Makefile, .circleci/config.yml and .circleci/*.sh.

Some tasks exist only on Core and are marked with ⊛, some exist only on a country package (France or Country Template) and are marked with ✣.

Task	CI config	Shell scripts	Makefile	OpenFisca CLI
Install dependencies	pip install --editable (optional)		make deps, make install	pip install openfisca_country_package
Lint		lint-changed-python-files.sh, lint-changed-yaml-tests.sh	make check-syntax-errors check-style
Apply linting rules			make format-style
Run tests	pytest && openfisca test		make test	openfisca test
Start the web API			make api (in Core) make serve-local (in Country Template)	openfisca serve
Check version number validity	git fetch && check_version_and_changelog.sh	check_version_and_changelog.sh
Build package			make build
Clean installed packages			make uninstall, make clean
Test web API ✣		test-api.sh
Deploy ✣	Deploy web API	publish-python-package.sh && publish-git-tag.sh
Deploy ⊛	Trigger documentation build update	publish-python-package.sh && publish-git-tag.sh
Check NumPy compatibility ⊛	Check NumPy typing against latest 3 minor versions
Check types ⊛			make check-types
Run Country-Template tests ⊛	Run Country Template tests
Log test coverage ⊛	Submit coverage to Coveralls

Usage survey

In parallel to this RFC, in order to identify which tasks might be removed altogether from the list of existing ones, and which destination is least likely to disrupt existing uses, a survey has been created by @MattiSG to better understand the current usages. You are strongly encouraged to participate to it: share your current workflow.

The results of the survey are publicly available.

Process

This RFC aims at finding the best solution to the problems identified above. Contributors are invited to vote on the propositions below, which will be exposed each in their own post, through emoji reactions (:+1: to support an option, :-1: to reject it, :eyes: to indicate having read it and not supporting nor rejecting it). Contributors are also invited to ask for clarifications, suggest improvements to the propositions if that could change their stance on them, and contribute their own if they believe they can offer significant improvements.

In order to contribute a proposition, please make clear whether it is a new one, in which case you should give it a new number (e.g. “proposition 5”), or if it builds upon an existing one to add some detail, in which case you should suffix the original number with a letter (e.g. “proposition 3A”).

This RFC will close after one week (7 ⨉ 24 hours) without changes to the proposition set. At that time, votes will be counted and the most consensual proposition will be implemented.

[MattiSG]

Proposition 0: do nothing

The status quo is acceptable. It is not worth investing in changing something that works, the problems identified mostly concern maintainers, who are in a limited number and can learn to cope with that complexity and decrease it slowly when opportunities arise.

[MattiSG]

Proposition 1: converge on a task manager

All workflow steps are declared in one place, and referenced elsewhere as needed. This can be achieved through make or a more modern alternative such as invoke or a combination of tox / nox and poetry (cf. #1036 (comment)).

• All tests are declared as targets of that tool, scripts are migrated to it and exposed as $tool check-version-number and $tool has-functional-changes, for example.
• The documentation only mentions $tool install, $tool test, $tool build, $tool publish.
• Tests parallelisation is handled at task manager level, with the number of parallel runners specified as an option to the target.
• CI config only refers to these tasks.
• The CLI is mostly relegated to a cross-platform way to expose some OpenFisca features, but not as the main point of entry for end users, building upon maintainers' experience with it.

[MattiSG]

Proposition 2: consider GitHub Actions as the task manager

CI is the main way all tests are executed, and is the most precise way to orchestrate the entire workflow, relying on a well documented syntax. With the migration to GitHub Actions (#1030), we have the opportunity to use act to execute all or part of the workflow locally.

• Users and contributors use standard python tooling (pip install) and the CLI (openfisca test, openfisca serve) for their usual needs.
• CI config defines all tasks as “jobs”, with clear dependency paths that can be rendered visually (e.g. Switch from CircleCI to GitHub Actions country-template#115) and executed cross-platform.
• Maintainers and advanced contributors use act to execute CI targets for packaging, relying on Docker.

[MattiSG]

Proposition 3: CLI for users, task manager for contributors

The CLI is maintained and possibly improved for users who simply install OpenFisca as a dependency, through its Web or Python API. At the same time, contributors are directed to a task manager. CI config barely orchestrates the tasks declared in the task manager.

• All commands that can be achieved with the CLI, are.
• For maintainers’ ease of use, sugar can be added by wrapping commands exposed in the CLI in the task manager (i.e. $tool test would redirect to openfisca test, passing all options).
• The user documentation only mentions pip install, openfisca test, openfisca serve.
• The CONTRIBUTING only mentions $tool install, $tool test, $tool build.
• Scripts are migrated to the task manager with $tool check-version-number and $tool has-functional-changes, for example.
• Tests parallelisation is handled at CLI level, with number of parallel runners specified as an option.
• CI config refers in priority to the CLI and, when a command does not exist in it, to the task manager targets.

[benjello]

All workflow steps are declared in one place, and referenced elsewhere as needed. This can be achieved through make or a more modern alternative such as invoke or a combination of tox / nox and poetry (cf. #1036 (comment)).

make is not easily available on Windows.
tox, nox and poetry are very nice tool but the learning curve for non expert (including myself ;-)) might be rough.
invoke seems a good compromise since it is python based and may be used in interaction with the CLI.

[bonjourmauko]

I'm pretty much in favour of proposition 3 :)

[MattiSG]

@maukoquiroga thanks for your enthusiasm! 😃 In order to minimise noise though, I encourage all participants to focus on using emoji reactions to express their preferences, and to use comments to add details and counter-propositions, as done above 🙂 Thank you, and sorry for being a killjoy 😉

[MattiSG]

Reminder: this RFC will close in 48 hours, unless the proposition set changes 🙂 Please cast your vote!

[HAEKADI]

Just fyi I tested Act on Openfisca France and it did not bode well for me personally nor for @sandcha. Here is a summary.

[bonjourmauko]

Thanks @MattiSG! So to add details as what my perfect solution could look like (mostly proposition 3 but a bit of 1):

Installing, building, etc.

$ make install

[⚙] Install project dependencies...

The CLI

Let's call it openfica, example usages:

$ openfica --help

Usage: openfica <subcommand> [--subcommand-opts] ...

Subcommands:

  serve                      Run the OpenFisca Web API.
  test                       Run OpenFisca tests.
  make.api                   Serve the openfisca Web API.
  make.check-style           Run linters to check for syntax and style errors.
  make.check-syntax-errors   Compile python files to check for syntax errors.
  make.check-types           Run static type checkers for type errors.
  make.clean                 Delete builds and compiled python files.
  make.format-style          Run code formatters to correct style errors.
  make.test                  Run openfisca-core tests.

$ openfica --help serve

Usage: openfica serve [--options]

Docstring:
  Run the OpenFisca Web API.

  Examples:
      $ openfica serve --country-package openfisca_country_template

Options:
  -c STRING, --country-package=STRING   Country package to use.
  -e [STRING], --extensions[=STRING]    Extensions to load.

$ openfica serve --country-package openfisca_country_template

[2021-09-08 21:08:23 +0200] [56907] [INFO] Starting gunicorn 20.1.0
[2021-09-08 21:08:23 +0200] [56907] [INFO] Listening at: http://127.0.0.1:5000 (56907)
[2021-09-08 21:08:23 +0200] [56907] [INFO] Using worker: sync
[2021-09-08 21:08:23 +0200] [56939] [INFO] Booting worker with pid: 56939
[2021-09-08 21:08:23 +0200] [56940] [INFO] Booting worker with pid: 56940
[2021-09-08 21:08:23 +0200] [56941] [INFO] Booting worker with pid: 56941

$ openfica --help test

Usage: openfica test [--options]

Docstring:
  Run OpenFisca tests.

  Examples:
      $ openfica test "$(git ls-files 'tests/**/*.py')"

Options:
  -p STRING, --path=STRING      Paths (files or directories) of tests to execute.
  -w STRING, --workers=STRING

$ openfica test "$(git ls-files 'tests/**/*.py')"

.................................................................................................................................................................................................... [ 40%]
....................................................................................................................................................................x............................... [ 81%]
..........................................................................................                                                                                                           [100%]
481 passed, 1 xfailed, 151 warnings in 6.67s

The task manager

openfica also provides a make-like API:

$ openfica make.clean make.check-syntax-errors make.check-style make.check-types make.test

[⚙] Delete builds and compiled python files...
[⚙] Compile python files to check for syntax errors...
[⚙] Run linters to check for syntax and style errors...
[⚙] Run static type checkers for type errors...
Success: no issues found in 131 source files
[⚙] Run openfisca-core tests...
.................................................................................................................................................................................................... [ 40%]
....................................................................................................................................................................x............................... [ 81%]
..........................................................................................                                                                                                           [100%]
481 passed, 1 xfailed, 151 warnings in 6.84s

Parallelisation

Good old make to the rescue:

## Launch all `openfica` tasks.
all: \
	openfica-clean \
	openfica-check-syntax-errors \
	openfica-check-style \
	openfica-check-types \
	openfica-test \
	;

openfica-%:
	@openfica make.$*

And then ✨ :

$ time make -j 1

[⚙] Delete builds and compiled python files...
[⚙] Compile python files to check for syntax errors...
[⚙] Run linters to check for syntax and style errors...
[⚙] Run static type checkers for type errors...
Success: no issues found in 131 source files
[⚙] Run openfisca-core tests...
.................................................................................................................................................................................................... [ 40%]
....................................................................................................................................................................x............................... [ 81%]
..........................................................................................                                                                                                           [100%]
481 passed, 1 xfailed, 151 warnings in 6.71s

real	0m13.114s
user	0m17.181s
sys	0m2.432s

$ time make -j `nproc`

[⚙] Compile python files to check for syntax errors...
[⚙] Delete builds and compiled python files...
[⚙] Run openfisca-core tests...
[⚙] Run linters to check for syntax and style errors...
[⚙] Run static type checkers for type errors...
Success: no issues found in 131 source files
.................................................................................................................................................................................................... [ 40%]
....................................................................................................................................................................x............................... [ 81%]
..........................................................................................                                                                                                           [100%]
481 passed, 1 xfailed, 151 warnings in 7.92s

real	0m9.444s
user	0m20.110s
sys	0m3.200s

Like it?

You can try a working prototype here 🥳

[MattiSG]

Thanks everyone for your participation! ☺️

Conclusion

Proposition 3 is both the most consensual and the most supported, with 5 👍 and 0 👎 . On top, @HAEKADI's detailed exploration demonstrates that proposition 2 would be impossible to implement.

Thus, Proposition 3: “CLI for users, task manager for contributors” is adopted.

I'll lock this conversation in order to lock votes. Implementation choices will move to the open issues and PRs, and to new ones if some elements are not already covered 🙂

Survey learnings

The data from the survey contains some interesting learnings, which I'll highlight below. Feel free to examine it and share your own learnings! I suggest that contribution number 6 should be excluded from analysis as it contains several answers that are impossible (using the OpenFisca CLI to deploy and uninstall the package).

Two things that I learned:

1. If one uses make for one task, they are much more likely to use make for other tasks.
2. There is no significant difference in usage between Core contributors and country package contributors.

The distribution in usage shows that:

1. Installation is mostly done with pip.
2. When there is a CLI available, it is preferred over make.
3. When there is a make task available, it is preferably applied in CI.

I believe this should make us adjust slightly the proposal in that the CONTRIBUTING file should only reference the task runner when there is no CLI command already available.