[releases] Harmonize versions and fix PyPI publishing by BenjaminPelletier · Pull Request #1687 · interuss/monitoring

BenjaminPelletier · 2026-09-03T00:02:33Z

This PR primarily fixes the PyPI publishing configuration to use the interuss_monitoring package name rather than the unavailable monitoring package name, and to ensure the version used for publishing is valid (follows PEP440). To do this consistently, the PR also coalesces multiple disparate versioning scripts into a single get_versions.py. While doing this, I found that tag.sh is only used in an obsolete release workflow so I did not coalesce tag.sh and instead removed it, correcting the release procedure documentation to match current practices. The complexity of build.sh increases somewhat with these changes, so to avoid duplication, build_dev.sh functionality is merged into build.sh and build_dev.sh removed.

the-glu · 2026-09-03T09:07:15Z

+      -f monitoring/Dockerfile \
+      -t "${DOCKER_TAG}" \
+      --build-arg version="${VERSION_STR}" \
+      --build-arg commit_hash="${COMMIT_HASH}" \


This changed from HEAD to COMMIT_HASH, witch may include the -dirty flag (nit: and it's not an hash).

This is injected in urls by repo_url_of and generate invalid links.

Thanks; this was unintentional and just trying to put everything related to versions in get_version.py -- I've updated to use the full commit hash (in to-be-pushed commit).

the-glu · 2026-09-03T09:42:49Z

+        help=(
+            "Explicit output format.\n"
+            "  pep440: Canonical PEP440 version (e.g., '0.31.0', '0.31.0rc1', '0.31.0+gd56bb4d.dirty'); fails for malformed pre-releases (-RC, -1.2, etc.).\n"
+            "  imagetag: docker image tag version (e.g., 'v0.31.0', 'v0.31.0-d56bb4d417-dirty').\n"


Imagetags don't have -rc suffix, but pep ones does. Shouldn't it be consistent?

This PR was previously trying to be consistent with what we had before, but I agree the rc suffix is probably good to have for the image tag; will adjust.

the-glu · 2026-09-03T09:47:51Z

-tag_regex = "^interuss/monitoring/v(?P<version>\\d+\\.\\d+\\.\\d+)"
-
+# Strictly anchor to the repo-level prefix, 3-part numeric PEP440/SemVer, and optionally PEP440 lowercase '-rcN' suffixes.
+tag_regex = "^interuss/monitoring/v(?P<version>\\d+\\.\\d+\\.\\d+(?:-rc\\d+)?)$"


setuptools_scm don't have the same behavior as the rest. Should just the script be re-used?
(if I uv build I get different results, on my fork it thinks its release 0.28.0, where was get_version.py use the remote and use different tags)

I've now completely removed setuptool_scm; its pretend versions were very confusing to me and I think the new approach is clearer (and reuses get_version.py).

Yes that probably a good thing, when preparing the package I used it because it is used in other interuss repos, but it needed to much workarounds.

the-glu · 2026-09-03T09:54:43Z

 ## Background

-Releases of monitoring are based on git tags in the format `interuss/monitoring/v[0-9]+\.[0-9]+\.[0-9]+`, optionally suffixed with `-[0-9A-Za-z-.]+`.  This tag form follows the pattern `[owner]/[component]/[semantic version]`; see [semantic version](https://semver.org) for more information.
+Releases of `monitoring` utilize Git tags structured precisely as `interuss/monitoring/v[X].[Y].[Z]`, optionally accompanied by strict pre-release/candidate identifiers adhering to **PEP 440 Pre-Release Conventions**. 


Are they? There are '-' in some situation described bellow, witch don't strictly follow PEP440 ? (They are allowed, but that not the normalized form')

I've clarified.

the-glu · 2026-09-03T10:16:25Z

+*   **Release Candidates (`vX.Y.Z-rc[N]`)**: **Strict Pre-Release Identifiers**. 
+    *   Pre-release tags **must** utilize the strictly lowercase, hyphenated `-rc[N]` suffix (e.g., `v0.31.0-rc1`). 
+    *   Build tooling normalizes these delimiters into PEP 440-compliant pre-release strings (`X.Y.Zrc[N]`) for PyPI compatibility.
+    *   Note: Pre-releases containing uppercase identifiers (e.g., `-RC1`), space delimiters, alphabetic metadata, or non-numeric suffixes (e.g., `-alpha`, `-1.2`) are prevented to avoid accidental malformed PyPI publication.


Nit, but that not very correct: -RC1 is a valid PEP440 format, -alpha as well. Only -1.2 is invalid, PyPI will accept (and normalize) the rest.

We probably want to restrict tags to a common format, but that the same for the docker image, and that not a PEP/PyPI requirement?

-RC1 is not in PEP440 canonical format to be used in public version identifiers, nor is -alpha.

Yes, but build system normalize it, malformed PyPI publication is not possible (and would reject it - same logic as having + to prevent submission):

cat pyproject.toml ─────┬─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── │ File: pyproject.toml ─────┼─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── 1 │ [project] 2 │ name = "testmcuorb" 3 │ version = "0.0.1-RC2" 4 │ description = "Test project" 5 │ requires-python = ">=3.9" 6 │ 7 │ [build-system] 8 │ requires = ["hatchling"] 9 │ build-backend = "hatchling.build" ─────┴─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── maximilien@pluna  ~/tmp/testpypi  uv build Building source distribution... Building wheel from source distribution... Successfully built dist/testmcuorb-0.0.1rc2.tar.gz Successfully built dist/testmcuorb-0.0.1rc2-py3-none-any.whl

the-glu · 2026-09-03T10:17:30Z

+*   **Interim Import Namespace (Phase 1 Phase-in)**: 
+    *   Consumers install the package via `pip install interuss_monitoring`.
+    *   In the current structural phase (Phase 1), internal code modules and external users importing from the PyPI package interact with the Python Import Namespace via **`import monitoring.<submodule>`**. 
+    *   *Warning for Interim Consumers*: Users must ensure their active Python virtual environment does not contain a conflicting top-level `monitoring/` directory from alternative third-party packages to prevent Python import-shadowing and runtime `ModuleNotFoundError` conflicts.


nit: the whole style of the doc is a bit off (things like "Interim Consumers")

"Interim Consumers" seems fairly clear to me: consumers of the package while it still using monitoring for imports rather than interuss_monitoring. But, I'll see if I can adjust any way.

the-glu · 2026-09-03T10:25:53Z

+    # If this is not an exact match on the release tag, or if workspace is dirty, append local version
+    metadata_segments = []
+    if not is_exact_tag_boundary:
+        metadata_segments.append(f"g{commit_hash.lstrip('g')}")


What the 'g' ? It cannot be present in a hash ?

Without --abbrev=0, describe returns something like interuss/monitoring/v1.2.0-4-g2a3b4c5. But the versioning logic is apparently very difficult even when factored out like this, so I'll work to make it as crystal clear as possible.

…and fix docker tag delimiters

BenjaminPelletier · 2026-09-04T03:47:25Z

This PR now has substantial changes after the previous round of comments.

the-glu · 2026-09-07T08:10:11Z

+
+    # Retrieve whether commit is local-only
+    cherry_res = run_git_cmd(["cherry"], check=False)
+    kwargs["is_localcommit"] = cherry_res.returncode == 0 and cherry_res.stdout.strip()


This return a string instead of bool, witch may create unwanted behavior later since it's declared as a bool.

python Python 3.14.7 (main, Aug 10 2026, 07:46:56) [GCC 16.1.1 20260728] on linux Type "help", "copyright", "credits" or "license" for more information. >>> True and "a" 'a'

the-glu · 2026-09-07T08:12:02Z

+    env_version = os.environ.get("MONITORING_VERSION")
+    if env_version:
+        return env_version


Nit: This is already done in get_pep440_version who is the only caller, this will never happen.

Functions must be encapsulated independent of their usage, so what calls this function shouldn't matter (because, for instance, we could add another caller later without being aware of the expectation the caller was supposed to do something) -- instead, it only matters what this function should do. When deriving the PEP440 version given observed GitInfo, it seems reasonable to check the magic environment variable. Or, we could perhaps change GitInfo to SystemInfo or something like that which could include the information of the environment variable value, but I think that would be a different solution that I'm not sure would be better.

the-glu · 2026-09-07T08:20:17Z

+build/lib/
+build/bdist*/


Are those two still needed?

uv build on my machine don't write to build/, but I know you have restrictions with your python setup.

uv build --wheel leaves these artifacts.

Ok, should we move them out then? build is used for other purposes, having it mixed with others artifacts may be an issue no?

I'm not sure what the suggested action is

diff --git a/pyproject.toml b/pyproject.toml index aea1a74cd..984bb0152 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -130,3 +130,6 @@ namespaces = true [tool.setuptools.dynamic] version = {attr = "monitoring._version.__version__"} + +[tool.distutils.build] +build_base = "buildpip"

(With the name to be revised, really not sure about it)

So everything goes to a different directory than the one we use for others scripts.

(It should probably be git ignored as well)

the-glu · 2026-09-07T08:28:45Z

This PR now has substantial changes after the previous round of comments.

Thanks! Found a few extra nits, but overall it looks good. I can't resolve the old threads on my side, so I'll let you clean them up.

mickmis

LGTM, I think. This was pretty challenging to review and I'm not sure I managed to be exhaustive there. Now, the impact in case there is an issue is also relatively limited (nothing that would blow up in prod, only inconveniences for devs), so that is probably OK. However I think a low hanging fruit to mitigate that would be to add unit tests to get_version.py.

mickmis · 2026-09-09T06:50:23Z

+    """True when the current state lies exactly on a valid tag (and therefore is suitable for release)."""
+
+    full_commit_hash: str
+    """Full SHAA-1 commit hash."""


Suggested change

"""Full SHAA-1 commit hash."""

"""Full SHA-1 commit hash."""

mickmis · 2026-09-09T07:10:35Z

+def get_git_info(component: str) -> GitInfo:
+    """Interrogates git for information relevant to versioning."""
+
+    kwargs: dict[str, Any] = {"component": component}


Create a GitInfo to benefit from the interpreter and typing?

Can't instantiate a dataclass without specifying all its required fields. Specifying all fields in the constructor will add a line per field (instead of GitInfo(**kwargs), it will be GitInfo(component=component, upstream_owner=upstream_owner, ...), but I can do that for easier typing.

mickmis · 2026-09-09T07:21:35Z

The functions in this file would benefit from having unit tests: there is a large combination of outcomes and ensuring those are all correct and will remain correct in the future is pretty hard.

Harmonize versions and fix PyPI publishing

d24f4e0

github-project-automation Bot added this to InterUSS Backlog Sep 3, 2026

BenjaminPelletier marked this pull request as ready for review September 3, 2026 00:49

the-glu reviewed Sep 3, 2026

View reviewed changes

Fix commit_hash build arg to full commit hash

129f287

BenjaminPelletier marked this pull request as draft September 3, 2026 18:29

BenjaminPelletier added 3 commits September 3, 2026 22:54

Address comments

0ccad10

Remove setuptools_scm

e4aadaa

Fix package inclusion in wheel, exclude build/lib from basedpyright, …

bbf8e86

…and fix docker tag delimiters

BenjaminPelletier marked this pull request as ready for review September 4, 2026 03:47

the-glu reviewed Sep 7, 2026

View reviewed changes

BenjaminPelletier added 3 commits September 8, 2026 18:15

Merge remote-tracking branch 'interuss/main' into harmonize-versions

7599426

Ensure boolean value

d18d73d

make format

2c827f7

mickmis approved these changes Sep 9, 2026

View reviewed changes

mickmis mentioned this pull request Sep 9, 2026

Update setuptools-scm requirement from >=10.2.1 to >=10.2.2 #1695

Closed

BenjaminPelletier marked this pull request as draft September 9, 2026 18:29

		build/lib/
		build/bdist*/

Uh oh!

Conversation

BenjaminPelletier commented Sep 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

the-glu Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

the-glu Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BenjaminPelletier commented Sep 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

the-glu commented Sep 7, 2026

Uh oh!

mickmis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

the-glu Sep 3, 2026 •

edited

Loading

the-glu Sep 7, 2026 •

edited

Loading