Repository navigation
Conversation
- build the Docker image from the prebuilt wheel in dist/ - add a safety guard to fail early if the wheel is missing - validate Docker images in CI without pushing from build.yml - expose head_sha for release metadata tracking - publish main-branch Docker images from the release workflow - use artifact download + imagetools promotion instead of rebuild-on-release
- document that the reusable build workflow validates images without pushing - clarify the required dist/ wheel prerequisite for local Docker builds - add a source-image existence check before release tag promotion - explain the main-branch GHCR publication flow and semver promotion order - align contributor docs with the actual Docker release behavior
|
Moving |
I debated about Thanks for the suggestion. I'll look into updating it when I get a chance. |
|
Pushed some changes that now keeps the Example: https://github.com/amimas/gitlabform/pkgs/container/gitlabform/versions |
|
I think the manual (workflow_dispatch) release silently skips Docker publication. publish-to-ghcr-release depends on publish-to-ghcr-main, which only runs on workflow_run, so the release ships to PyPI and GitHub with no image tags and no failure. I think the published images also lose all OCI labels now that nothing applies the metadata-action output, and the Dockerfile's wheel glob breaks (or silently ships stale code) as soon as dist/ holds more than one wheel locally. Also there are some loose ends: commented-out code and duplicate logging in dev/docker.py, an advertised extra_args that is silently dropped, an error message pointing to a command that doesn't exist, and docs describing behaviour the workflow doesn't have (latest moving on main builds, vX.Y.Z tags). Could you give the whole PR a cleanup pass with this in mind? I will re-review after that. Ow, little note, one thing to decide explicitly. The release flow can no longer build an image itself, so if the main-branch run never pushed the sha image, a manual release can't recover. Is that intended? |
|
Really good observation @rickbrouwer. I think they are valid issues. I missed those! I'll look into those issues, soon I hope.
You're right. I think we have to get the The main objective for the PR was that same artifact should be used everywhere. The python package gets built and verified in both PR and So, this does make the release flow dependent on At the same time, I've been thinking about our acceptance tests. I think they do have value and we're able to catch issues with GitLab's API change quickly. But the tests are growing and taking longer - some has sleep and retry logic. Right now I think acceptance tests takes about 30 minutes. I haven't thought it through yet, but maybe the acceptance tests needs to be slimmer and only test individual API calls instead of every possible scenario. For example: Question: Should the docker image publish be dependent on pypi release? It's just a made up dependency that exists today - maybe just to make sure pypi release is successful. I considered whether docker build should install from pypi, but decided against it because that means in PR, we can't easily build and verify the docker image and I don't like the idea of different process between PR vs main branch workflow. Please let me know if you have more suggestions or whether we/I should continue with this PR. Thanks again for reviewing this, btw. Really appreciate it. |
I am strongly in favor of building to 'main' in that case.
I haven't yet closely examined whether we have any truly redundant or duplicate tests, but I am a proponent of comprehensive integration tests that provide thorough coverage. Admittedly, this can result in very long execution times. On another project where I work, we address this by using regex to select which tests to run; for instance, you can launch a test using
I'd keep it. Since both now use the same wheel, the dependency is no longer technical, but it still guards release consistency. A PyPI version can never be re-uploaded, while Docker tags can be re-promoted at any time. If PyPI fails, a vX.Y.Z image would exist for a version that may never ship on PyPI. Promotion only retags, so the extra wait is negligible. Agree on not installing from PyPI: same artifact and same process in PR and main is the right call. |
Oh right, I forgot to reply to this one. |
This PR refactors the container build and release process.
Currently the CI process does not have any steps for building the container image. It only gets built after changes are merged into
mainbranch. Even then, the container image is created by rebuilding the python package/wheel.This PR introduces following changes regarding how container image is built:
Dockerfiledoes not build the python package - it consumes a pre-built package. This makes both pypi release and container image using the same artifact created in build stepBuilding on those, updated when container image is published. Every merge to
mainbranch will publish docker image using 2 non-versioned tags. One tag issha-<short-commit-hash>and another forsha-<full-commit-hash>. This way, features/fixes are readily available for anyone to test without needing to wait for a release to happen in gitlabform. Also addedmaintag point to these commit based tags. This tag will auto update on every merge to main branch.All other release process stays same as before. The gitlabform version can be a bit confusing from the commit based or non-versioned tags. Wanted to limit the amount of changes being introduced in this PR. So, didn't look into this as currently version numbers are hard coded in
pyproject.tomlandtbump.tomlfiles. This is probably a different refactor for later.Tested the change in my fork repo. The docker image tags can be seen here and an example of the release workflow run here.