andrewmusselman opened a new pull request, #1264:
URL: https://github.com/apache/tooling-trusted-releases/pull/1264
# Track the staging revision of a recorded distribution
Closes part of #751. Related: #938.
## Summary
Two small, independent changes to the distribution layer. Both fall out of
andrewmusselman's RFC on #751 as "independently useful regardless of whether
remote promotion is ever built."
**A nullable `staging_revision_key` foreign key on the `distribution`
table.** This records which `Revision` was the source of a staged distribution.
The original issue calls out the need directly: *"we have to check that the
files being promoted are the same as those that were voted on."* Without a
record of the connection between a staged distribution and the underlying
revision, ATR has no way to enforce that — for local re-upload today, or for
remote promotion later. The column is the foundation that any future
verification check can stand on. It is nullable so existing rows and call sites
that don't yet know the revision key remain valid; a `TODO(#751)` in
`CommitteeMember.__upgrade_staging_to_final` marks where the assertion will
live once call sites populate the field.
**Removal of the hard-coded `{ARTIFACT_HUB, PYPI, MAVEN}` whitelist in
`shared.distribution._template_url`.** That set duplicated knowledge already
encoded in `DistributionPlatformValue.template_staging_url`. The two had to be
edited in lockstep, which is a quiet footgun: adding a new staging URL (for
example, enabling the commented-out GITHUB platform in `sql.py`, or eventually
a Hex platform for #938) would silently fail until someone remembered to update
the parallel set. After this change the enum is the single source of truth — a
platform supports staging iff its value declares a `template_staging_url`. The
error message also now names the offending platform, which makes the failure
mode obvious.
## Files changed
| File | Change |
| --- | --- |
| `atr/models/sql.py` | Add nullable `staging_revision_key: str \| None` FK
column to `Distribution` |
| `atr/shared/distribution.py` | Remove `{ARTIFACT_HUB, PYPI, MAVEN}`
whitelist in `_template_url`; the error message now names the platform |
| `atr/storage/writers/distributions.py` | Thread `staging_revision_key`
through `record()` and `record_from_data()` as an optional pass-through; add
`TODO(#751)` marker in `__upgrade_staging_to_final` |
| `migrations/versions/0086_2026.05.21_38b1cb95.py` | Alembic migration:
nullable column + FK; reversible via `downgrade()` |
| `tests/unit/test_distribution_staging.py` | Six tests covering
`_template_url` for every platform (regression-pinning that no whitelist gets
reintroduced) and `Distribution.staging_revision_key` defaults |
## Smoke tests run
All four of these were exercised locally against `docker compose up --build`
on this branch. Anyone reviewing can reproduce them in a few minutes.
**1. Schema and migration.** On server boot, ATR's automatic `alembic
upgrade head` plus the follow-up `alembic check` (in `atr/db/__init__.py`)
validates that the migration applies cleanly and that the resulting DB schema
matches the SQLModel definitions. Boot succeeded with `Alembic check passed: DB
schema matches models`. To re-verify locally: `docker compose up --build` and
watch for that line in the logs.
**2. Whitelist removal does not regress staging for platforms that worked
before.** Navigate to `/distribution/stage/record/<project>/<version>` and
submit the form with a non-existent package on each of the three platforms that
have a staging URL. The expected result is an upstream 404 (or similar parse
error) — not the old whitelist message.
- **ArtifactHub** (`helm`, `asfowner`, `example`, `1`): `404 Not Found` from
`https://staging.artifacthub.io/api/v1/packages/helm/asfowner/example/1` ✅
- **Maven Central** (`orgapacheexample`, `example`, `1`): `404 Not Found`
from
`https://repository.apache.org:4443/repository/maven-staging/orgapacheexample/example/maven-metadata.xml`
✅
(Maven groupIds are dotted in the real world, but the form's
`owner_namespace` field is typed `safe.Alphanumeric` which rejects `.`. That's
a pre-existing form-validation issue, unrelated to this PR, and worth filing
separately.)
**3. New error message names the platform.** Submit the same form with
**Docker Hub** (`library`, `nginx`, `latest`). Expected: `Platform Docker Hub
does not provide a staging API endpoint.` ✅
The `RuntimeError` raised by `_template_url` propagates to a 500 page, but
that propagation is pre-existing — the old whitelist branch raised the same
`RuntimeError` and produced the same 500. Only the message text changed. The
500-wrapping itself is worth a separate UX issue.
**4. New column is present and behaves as expected.**
```shell
sqlite3 state/database/atr.db "PRAGMA table_info(distribution);" | grep
staging_revision_key
```
returns the column, nullable, FK to `revision.key`. After recording any
distribution through the UI, `SELECT staging_revision_key FROM distribution`
returns `NULL` for every row — expected, since this PR deliberately does not
wire call sites to populate the field. That's a follow-up.
## What this deliberately does not change
The RFC laid out several follow-ups and called for a "narrow rather than
generic" implementation. This PR matches that posture.
- **No `supports_promotion` / `promotion_mechanism` columns.** YAGNI until a
second platform actually needs them. `supports_promotion: bool` is derivable
from `promotion_mechanism is not None`, and `promoted: bool` / `promoted_at`
duplicate state that the existing `staging` boolean flip already encodes.
- **No decision on the commented `GITHUB` enum.** That needs a human yes/no.
The whitelist removal here means a future enable just works.
- **No automatic revision-key lookup at call sites.** The new parameter on
`record()` and `record_from_data()` is optional and defaults to `None`. Wiring
`post/distribution.py`, `tasks/distribution.py`, and `api/__init__.py` to look
up the current `Revision` and pass it through is a small follow-up that becomes
meaningful now that the column exists.
- **No verification assertion in `__upgrade_staging_to_final`.** The
`TODO(#751)` marker shows where it lands; the assertion follows once call sites
populate the field.
- **No Hex/Erlang (#938) work.** That's blocked upstream on
`hexpm/hexpm#1193` and needs its own design. The whitelist removal does make a
future Hex platform easier to add.
- **No investigation document in `atr/docs/`.** Better handled as a separate
PR once the team decides where investigation docs live.
## Required acknowledgements
- [x] I have read CONTRIBUTING.md.
- [x] "Allow maintainer edits" is enabled.
- [x] No new dependencies are introduced.
- [x] No `# noqa` or `# type: ignore` added.
- [x] Commit message uses imperative mood and is under 72 chars on the first
line.
- [x] `make check`, `sh tests/run-unit.sh` pass locally.
- [x] Migration round-trips cleanly (`alembic upgrade head` → `downgrade -1`
→ `upgrade head`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
