cwacek/python-jsonschema-objects

CI experimental version logic duplicates job

EliahKagan opened this issue · 0 comments

The pythonpackage.yml workflow, which provides linting and testing jobs, attempts to set continue-on-error to true for the latest tested version, so that failures in that version do not cause other jobs generated from the same matrix to be canceled. Unfortunately, the approach it uses does not succeed: while an "experimental" job is created, a non-"experimental" job is also created for the same version. The duplication can be seen in any recent workflow runs.

The impact is currently minimal, but is likely to become more severe if/when 3.12 is added and made the experimental version. Even before that, fixing it would improve the clarity of CI output.

The cause of the duplication

The workflow currently uses an approach similar to the pattern shown in the GitHub Actions documentation, but:

  • The python-version and experimental lists in the CI matrix definition generate a job for it with experimental=false.
  • The include list that augments the CI matrix generates a separate job for it with experimental=true, because when an item in the include list introduces a value that differs from an existing job's value for the same key, it creates a new job rather than modifying the existing job.

Current and future effects of the bug

If the effect were just an extra job that behaved identically, then this issue would be extremely minor. The deeper problem is that, because a test failure would cause both jobs to fail, the failure of the experimental=false job would still cancel other jobs generated from that matrix, including the jobs for other Python versions.

This doesn't really cause problems now, since the current "experimental" version is 3.11, which this project supports very well. But it is likely to cause problems if/when the "experimental" version is changed to 3.12, which is something that may make sense to do in the very near future.

Possible solutions

  1. One approach would be to simply remove the latest version from the python-version: [...] list, while still listing it under include:, since that is sufficient to put it in the matrix. A possible benefit is that this is a very small change, and it is the approach the documentation shows in its example. However, this has the disadvantage that it would not be immediately obvious to human readers what versions are tested. (The version could even end up being re-added to the test matrix as part of a larger change, reintroducing the bug.)

  2. Since the latest version currently listed is 3.11, the experimental/continue-on-error logic could be removed from the workflow entirely, since this library's support for 3.11 is now well established. But I don't recommend this solution, because Python 3.12 shall soon be released (it has a release candidate already), and experimental/continue-on-error logic would likely be brought back for that. Furthermore, if 3.12 is tested on CI while still a release candidate, then an experimental key (with a correct value) helps with that too, since it can be passed as the value of the setup-python action's allow-prereleases option.

  3. While items in an include list don't override different values that the matrix already had (they generate new jobs instead), they do override different values appearing in less specific items in the include list itself. Therefore, all versions can be listed together without unintended duplication, if both the false and true values for experimental are given in items under include. (Then which version is treated as experimental can be changed from 3.11 to 3.12 when 3.12 is added, without requiring further changes at that time to how the matrix is defined.) It seem to me that this approach is best, and I've proposed it in #249.