Initial attempt at GitHub Actions based CI. #199
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
|
We're green! (with lowered standards). I am going to take a shot at updating the Vagrantfiles alongside the addition of earthly but I'm tempted to go ahead with this even if it only contributes to my own maintenance of this package. |
nuclearsandwich
force-pushed
the
actions-ci
branch
3 times, most recently
from
2a39f87
to
cf542af
Compare
<https://earthly.dev> is an open source (MPL-2.0) project for defining and executing builds using containers. It meets a lot of the same needs as the Vagrant boxes that are in this project using containers rather than VMs. The advantage of pairing this with GitHub Actions is that it allows maintainers and contributors to reproduce test results locally which is very difficult when using GitHub Actions natively. stdeb interacts with a lot of system-level utilities so having convenient testing across multiple supported distributions is highly valuable.
Since we have the earthly configuration for local testing, we may as well use it for GitHub Actions as well. Use the matrix support to pass the OS build argument to Earthly. Keep platforms that aren't working yet disabled.
* Use python3 for running packaging tests. * Update version of psycopg2 used for tests. I got build failures on bullseye with the earlier version but success with this last one (the psycopg project apparently returned to the psycopg module for v3).
Right now this only sets a preferred line-length limit. It's possible this will be folded into a pyproject.toml as part of other updates to the project.
Travis CI has long since stopped providing value to open source projects and I believe that the Earthly builds cover the same local testing niche as the Vagrant files, which I was not able to successful update. If another maintainer or contributor wants to restore the Vagrant support I will help as I can, but I don't have a working vagrant provider locally.
Avoid copying in .git and other unecessary files by specifying source files explicitly. To make this slightly less unfun factor it into a function.
nuclearsandwich
force-pushed
the
actions-ci
branch
from
f130b43
to
1c52e88
Compare
With the addition of #203, a missing Python 2 binary will raise an error instead of assuming that the binary is called `python`. As a result, this test script now fails on platforms where no Python 2 installation is available.
j-rivero
reviewed
Nov 14, 2024
There was a problem hiding this comment.
Looks great to me. Some issues or questions:
-
Looking into the platforms supported by travis before, the ppc64le was an option, not sure if we can include it in the new setup or it was a conscious decision to remove it.
-
The lint action succeeded but seems to have a couple of errors from ruff:
+lint | 17 files would be reformatted, 2 files already formatted
2024-11-13T23:04:18.5015532Z +lint | error: Failed to parse test_data/py2_only_pkg/py2_only_pkg/py2_module.py:4:15: Unparenthesized tuple expression cannot be used here
2024-11-13T23:04:18.5080429Z +lint | --> RUN ruff check || true
2024-11-13T23:04:18.5772002Z +lint | stdeb/cli_runner.py:37:21: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
2024-11-13T23:04:18.5772739Z +lint | |
2024-11-13T23:04:18.5773088Z +lint | 35 | for option in optobj.__dict__:
2024-11-13T23:04:18.5773572Z +lint | 36 | value = getattr(optobj, option)
2024-11-13T23:04:18.5774013Z +lint | 37 | is_string = type(value) == str
2024-11-13T23:04:18.5774470Z +lint | | ^^^^^^^^^^^^^^^^^^ E721
2024-11-13T23:04:18.5774948Z +lint | 38 | if option in bool_opts and is_string:
2024-11-13T23:04:18.5775451Z +lint | 39 | setattr(optobj, option, strtobool(value))
2024-11-13T23:04:18.5775883Z +lint | |
2024-11-13T23:04:18.5776062Z
2024-11-13T23:04:18.5776570Z +lint | test_data/py2_only_pkg/py2_only_pkg/py2_module.py:4:15: SyntaxError: Unparenthesized tuple expression cannot be used here
2024-11-13T23:04:18.5777245Z +lint | |
2024-11-13T23:04:18.5777623Z +lint | 2 | """this is a function that works only in python 2"""
2024-11-13T23:04:18.5778066Z +lint | 3 | if False:
2024-11-13T23:04:18.5778803Z +lint | 4 | raise Exception, 'this is a SyntaxError in py3 but not 2' # noqa
2024-11-13T23:04:18.5779296Z +lint | | ^
2024-11-13T23:04:18.5779644Z +lint | 5 | return value+value
2024-11-13T23:04:18.5779981Z +lint | |
2024-11-13T23:04:18.5780157Z
2024-11-13T23:04:18.5780263Z +lint | Found 2 errors.
2024-11-13T23:04:18.5890792Z output | --> exporting outputs
Is this expect by the configuration?
build.earth
Outdated
| FUNCTION | ||
| ENV DEBIAN_FRONTEND=noninteractive | ||
| # Build deps | ||
| RUN apt-get update; apt-get install -y debhelper dh-python python3-all python3-pip |
There was a problem hiding this comment.
For performance reasons maybe use a single apt-get update and apt-get install invocation and if you want to keep the dependencies separated use env vars?
Good point. It wasn't conscious but I also don't know that there needs to be specific support for it in this pure-python package. It looks like Earthly can work with multi-platform docker images although I got some strange errors when trying it. I'll consider looking at it more in the future but for the sake of getting some CI running I won't block on it. |
Yeah, I think the errors are because there is Python 2 and 3-compatible syntax in use that ruff isn't okay with. So I can't fully enable the linters until Python 2 support is dropped in #198. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Travis has long since ceased being a viable CI platform for open source projects.
It's a pain to bake so heavily into yet another platform but at least it is right here.
I've had some success in other projects using https://earthly.dev to allow the same config to run locally and in CI so wrapping that in GitHub actions may lower the barrier to local testing for future contributors. For now I just want some CI feedback.
The use of
ruffinstead offlake8is entirely a curiosity since I haven't used before and I'm not at all committed to it.