Skip to content

Commit 1efbbe8

Browse files
committed
Merge remote-tracking branch 'origin/main' into pmm
2 parents 61e1246 + b97ee98 commit 1efbbe8

675 files changed

Lines changed: 56995 additions & 9450 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/instructions/migrations.instructions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ SEP has three independent Alembic migration tracks, one per sub-application. Eac
1717
CI runs `make checkmigrations` on every PR — it catches missing migrations and out-of-date heads automatically. Don't manually flag "model changed but no migration": CI already does. Focus on the manual-judgment items below.
1818

1919
- **Migration lives in the right track.** A model change under `app/sep/` MUST have its migration in `app/sep/migrations/versions/` (or under `app/sep/apps/<name>/migrations/versions/` for a sub-app that owns its own chain — those dirs are auto-listed in `[sep] version_locations` by `scripts/sync_alembic_version_locations.py`), NOT under `app/tasks/migrations/` or `app/inventory/migrations/`. Cross-track placement is a hard fail — the migration won't run on the right database.
20-
- **Deleting a sub-app's migration chain is an explicit step.** The sync script refuses to drop an entry already listed in `[sep] version_locations`, because that configured-but-absent path is how the orphan-head filter recognises a stripped app. A deliberate deletion needs `python scripts/sync_alembic_version_locations.py --allow-removals`; anything else (a locally stripped app, a half-deleted package) should restore the directory, or skip the script and run `alembic --name sep upgrade heads` directly — the refusal leaves the entry in place, which is exactly what arms the filter.
20+
- **Deleting a sub-app's migration chain is an explicit step.** The sync script refuses to drop an entry already listed in `[sep] version_locations`, because a configured location that is absent from disk or contributes no migration scripts is how the orphan-head filter recognises a stripped app. A deliberate deletion needs `python scripts/sync_alembic_version_locations.py --allow-removals`; anything else (a locally stripped app, a half-deleted package) should restore the directory, or skip the script and run `alembic --name sep upgrade heads` directly — the refusal leaves the entry in place, which is exactly what arms the filter.
2121
- **`upgrade()` and `downgrade()` are both implemented.** Empty `downgrade()` (just `pass`) is a red flag — migrations must be reversible.
2222
- **`downgrade()` reverses `upgrade()`.** New tables in `upgrade()` need `op.drop_table()` in `downgrade()`; new columns need `op.drop_column()`; new indexes need `op.drop_index()`. The pair must be symmetric. A `downgrade()` that re-narrows a column's allowed domain (removes enum members, re-adds `NOT NULL`, shrinks `VARCHAR` length) must first `DELETE`/`UPDATE` rows holding the now-invalid values — otherwise it aborts at runtime the moment such data exists.
2323
- **No data loss without a written strategy.** `op.drop_table()` or `op.drop_column()` on a column that has live data needs an explicit data-migration step (or a justification in the PR description). Same for column type changes that may truncate (`TEXT``INTEGER`, `VARCHAR(255)``VARCHAR(50)`). A parser branch or deserializer that silently remaps an old stored value at read time is NOT a substitute for a one-shot data migration (`UPDATE`/`DELETE`) — flag it.

.github/instructions/python-docstrings.instructions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ The reverse of overstating: when a docstring uses applicability language (`Use w
8383
- **Describe intent, not a code-controlled enumeration** — a docstring listing the members of a set the code controls (a `parametrize` list, a module constant, registry entries) goes stale the moment the set changes. Name the intent and the controlling symbol ("every app in `BESPOKE_BASE_APP_PLUGINS`"), not the members. Applies in `tests/` too.
8484
- **Framework/core symbols stay app-agnostic** — a shared framework/core symbol's docstring describes its behaviour in generic terms; naming one downstream app's domain concept (`backup_type`, `snippet_filename`) couples the abstraction's contract to one consumer. Restate as the generic role.
8585
- **State the role; don't pin what the reader can grep.** Two shapes rot on the next change and are never edited at the change site: **a count** ("read by seven non-alerts apps", "the only two callers") and **an enumeration of callers or consumers**. Write the *property* instead — not "read by seven non-alerts apps" but "read by every app offering `alert_on_fail`"; not "used by `a.py`, `b.py`, `c.py`" but "shared across the subtree". When the enumeration is genuinely load-bearing, the enforcement belongs in code (a registry, an `__all__`, a guard), with the docstring pointing at it — prose is not a mechanism.
86-
- **A parity claim is scoped to the members it covers.** "The expected row orders are the same literals `TestListQueryPaginatedPostgres` asserts" reads as a guarantee over the whole class; when it holds for only some members it is false for the rest and nothing marks which. Scope the assertion and name the exceptions ("…the all-NULL and `select_related` cases are MySQL-only"). Cross-dialect test classes are the recurring shape.
86+
- **A parity claim is scoped to the members it covers.** "The expected row orders are the same literals `TestListQueryPaginatedPostgres` asserts" reads as a guarantee over the whole class; when it holds for only some members it is false for the rest and nothing marks which. Scope the assertion and name the exceptions ("…the all-NULL case is PostgreSQL-only"). Cross-dialect test classes are the recurring shape.
8787

8888
## Say it once, in the surface that owns it
8989

.github/instructions/python-duplication.instructions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ Concrete cases: `3306`/`5432` ports → `DEFAULT_MYSQL_PORT`/`DEFAULT_POSTGRESQL
4242
| `len(v) > 0` `field_validator` | `NonEmptyStr` |
4343
| `.strip().lower()` `field_validator` | `Annotated[str, StringConstraints(strip_whitespace=True, to_lower=True)]` or `LowercaseStr` |
4444
| `field_validator` doing a string-*shape* check (split on a separator, reject empty halves, reject stray whitespace) | `Annotated[str, StringConstraints(pattern=...)]` field type |
45-
| `.nulls_last()` on an `ORDER BY` term | `app/core/db/utils.py::NullsLastOrdering(column, *, descending=False)``.nulls_last()` emits SQL MySQL cannot parse. Pass the bare column plus `descending=`, never a pre-`desc()`-ed expression |
45+
| `.nulls_last()` on an `ORDER BY` term | `app/core/db/utils.py::NullsLastOrdering(column, *, descending=False)`one shared cache-keyed construct; pass the bare column plus `descending=`, never a pre-`desc()`-ed expression (which would render `<expr> DESC ASC NULLS LAST`) |
4646

4747
**Rule of thumb.** If a new decorator or class has 15+ lines of state management (timestamps, eviction, key hashing, TTL math), ask "why isn't this `@alru_cache` or `@ttl_cache`?" Flag as **Important**.
4848

.github/labeler.yml

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,6 @@ frontend:
1313
- 'frontend/**'
1414
- 'package.json'
1515

16-
skip-test:
17-
- all:
18-
- changed-files:
19-
- any-glob-to-all-files:
20-
- 'CODEOWNERS'
21-
- 'README.md'
22-
- '.gitignore'
23-
- 'dist/**'
24-
- head-branch:
25-
- '^dependabot/'
26-
2716
# svc:* labels track the mounted services, which share a name with SEP apps but
2817
# are different code. The prefix keeps app/tasks/ (the service) distinct from
2918
# app/sep/apps/tasks/ (the SEP app); same for inventory.

.github/workflows/ci.yml

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ jobs:
3838
- name: Require QA approval label
3939
if: >-
4040
!contains(github.event.pull_request.labels.*.name, 'qa passed') &&
41-
!contains(github.event.pull_request.labels.*.name, 'skip-test')
41+
!contains(github.event.pull_request.labels.*.name, 'qa not required')
4242
run: |
43-
echo "::error::PR requires 'qa passed' or 'skip-test' label to merge."
43+
echo "::error::PR requires 'qa passed' or 'qa not required' label to merge."
4444
exit 1
4545
4646
# Detect changed file types to gate downstream jobs.
@@ -79,8 +79,13 @@ jobs:
7979
- 'sidecar/settings-env.sh'
8080
- 'sidecar/supervisord.conf'
8181
- 'sidecar/healthcheck.sh'
82+
- 'sidecar/entrypoint.sh'
83+
- 'sidecar/wait_for_schema.sh'
84+
- 'sidecar/Containerfile.sidecar'
8285
- 'Makefile'
8386
- 'frontend/packages/api/specs/**'
87+
- '.github/labeler.yml'
88+
- '.github/workflows/ci.yml'
8489
precommit:
8590
- '.pre-commit-config.yaml'
8691
- '.github/workflows/ci.yml'
@@ -301,6 +306,7 @@ jobs:
301306
test -f /home/sep/app/supervisord.conf
302307
test -f /home/sep/app/healthcheck.sh
303308
test -x /home/sep/app/entrypoint.sh
309+
test -x /home/sep/app/wait_for_schema.sh
304310
test -f /home/sep/app/settings-env.sh
305311
test -f /home/sep/app/settings.yaml
306312
'
@@ -330,6 +336,35 @@ jobs:
330336
- name: Smoke test - restricted image ships exactly the activated apps
331337
run: sidecar/verify_image_apps.sh "sep:HEAD" restricted
332338

339+
- name: Verify the purge layer is the last package-manager operation
340+
run: python3 scripts/check_sidecar_purge.py --check-ordering
341+
342+
- name: Smoke test - purged packages are absent from the built image
343+
run: |
344+
pkgs="$(python3 scripts/check_sidecar_purge.py --print-packages | tr '\n' ' ')"
345+
if [ -z "$pkgs" ]; then
346+
echo "::error::the purge checker named no packages, so this check cannot run"
347+
exit 1
348+
fi
349+
docker run --rm --entrypoint /bin/sh -e PURGED="$pkgs" "sep:HEAD" -c '
350+
command -v dpkg-query > /dev/null 2>&1 || {
351+
echo "::error::dpkg-query is absent, so package presence cannot be established"
352+
exit 1
353+
}
354+
status=0
355+
for p in $PURGED; do
356+
if dpkg-query -s "$p" 2>/dev/null | grep -q "^Status: install ok installed"; then
357+
echo "::error::$p is still installed in the built image"
358+
status=1
359+
fi
360+
done
361+
if command -v perl > /dev/null 2>&1; then
362+
echo "::error::a perl binary resolves on PATH"
363+
status=1
364+
fi
365+
exit $status
366+
'
367+
333368
pipeline-syntax:
334369
if: >-
335370
${{

.github/workflows/labels.yaml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,13 @@ jobs:
3030
sparse-checkout: |
3131
.github
3232
scripts
33-
- name: Compute blast-radius labels
33+
- name: Sync computed PR labels
3434
if: github.event_name == 'pull_request_target'
3535
env:
3636
GITHUB_TOKEN: ${{ github.token }}
3737
run: |
38-
python3 scripts/compute_blast_radius_labels.py \
38+
python3 scripts/sync_pr_labels.py \
3939
--owner "${{ github.repository_owner }}" \
4040
--repo "${{ github.event.repository.name }}" \
41-
--pr-number "${{ github.event.pull_request.number }}"
41+
--pr-number "${{ github.event.pull_request.number }}" \
42+
--head-ref "$GITHUB_HEAD_REF"

.github/workflows/python.yaml

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ jobs:
8080
- name: Install test dependencies
8181
run: poetry sync --no-root
8282
- name: Run tests
83-
run: make test PYTEST_MARKERS='not postgres and not mysql' COV=${{ matrix.python == '3.11.9' && '1' || '0' }}
83+
run: make test PYTEST_MARKERS='not postgres' COV=${{ matrix.python == '3.11.9' && '1' || '0' }}
8484
- name: Generate coverage comment data
8585
id: coverage_comment
8686
if: matrix.python == '3.11.9' && github.event_name == 'pull_request'
@@ -125,34 +125,6 @@ jobs:
125125
- name: Run PostgreSQL tests
126126
run: make test PYTEST_MARKERS='postgres' PYTEST_WORKERS=0 COV=0
127127

128-
test_mysql:
129-
name: test (mysql)
130-
runs-on: ubuntu-latest
131-
services:
132-
mysql:
133-
image: mysql:8.0.46
134-
env:
135-
MYSQL_ROOT_PASSWORD: sep
136-
MYSQL_DATABASE: sep_test
137-
ports: ["3306:3306"]
138-
options: >-
139-
--health-cmd "mysqladmin ping -h 127.0.0.1 -psep --silent"
140-
--health-interval 10s --health-timeout 5s --health-retries 10
141-
env:
142-
SEP_TEST_MYSQL_DSN: mysql+aiomysql://root:sep@127.0.0.1:3306/sep_test
143-
steps:
144-
- name: Checkout codebase
145-
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
146-
- name: Setup Python
147-
uses: ./.github/actions/setup-python-job
148-
with:
149-
python-version: "3.11.9"
150-
cache: poetry
151-
- name: Install test dependencies
152-
run: poetry sync --no-root --with mysql
153-
- name: Run MySQL tests
154-
run: make test PYTEST_MARKERS='mysql' PYTEST_WORKERS=0 COV=0
155-
156128
build:
157129
strategy:
158130
matrix:

.pre-commit-config.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,13 @@ repos:
5353
additional_dependencies: ["python-minifier==2.11.3"]
5454
files: ^app/sep/apps/(?!dipper/)(?:.*/)?(?:payload|[^/]*_payload)$
5555
pass_filenames: true
56+
- id: check-sidecar-purge
57+
name: Check side-car purge layer ordering
58+
description: "Ensure no apt/dpkg instruction follows the purge layer"
59+
entry: python3 scripts/check_sidecar_purge.py --check-ordering
60+
language: system
61+
files: ^(sidecar/Containerfile\.sidecar|scripts/check_sidecar_purge\.py)$
62+
pass_filenames: false
5663
- id: gen-xtrabackup-payload-variants
5764
name: Check xtrabackup payload variants
5865
description: "Ensure the generated xtrabackup payload variants match the canonical payload"
@@ -95,6 +102,7 @@ repos:
95102
hooks:
96103
- id: check-ast
97104
- id: check-case-conflict
105+
- id: check-added-large-files
98106
- id: check-json
99107
- id: check-merge-conflict
100108
- id: check-toml

0 commit comments

Comments
 (0)