Skip to content

Basic inventory - #6

Merged
cezmunsta merged 2 commits into
mainfrom
inventory
Dec 13, 2023
Merged

Basic inventory#6
cezmunsta merged 2 commits into
mainfrom
inventory

Conversation

@cezmunsta

Copy link
Copy Markdown
Contributor

API to read and store the inventory from a remote source. This is currently limited to PMM.

  • Added sep.inventory.api along with data models. The inventory is initialised from the remote source when the local data is emtpy. Additionally, the inventory can be refreshed by request. There is partial support to create and update items in the inventory directly via the API, only affecting local storage, which may be overwritten if changing an item that is present in the remote source

API to read and store the inventory from a remote source. This is
currently limited to PMM.

* Added sep.inventory.api along with data models. The inventory is
  initialised from the remote source when the local data is emtpy.
  Additionally, the inventory can be refreshed by request. There is
  partial support to create and update items in the inventory directly
  via the API, only affecting local storage, which may be overwritten if
  changing an item that is present in the remote source
@cezmunsta cezmunsta added the enhancement New feature or request label Dec 13, 2023
@cezmunsta cezmunsta self-assigned this Dec 13, 2023
@cezmunsta
cezmunsta marked this pull request as ready for review December 13, 2023 13:35
@cezmunsta
cezmunsta merged commit 72587ee into main Dec 13, 2023
@cezmunsta
cezmunsta deleted the inventory branch December 13, 2023 13:45
nachodd added a commit that referenced this pull request May 19, 2026
…#738)

## Summary

- Adds `useUnsavedChangesGuard` hook (`hooks/useUnsavedChangesGuard.ts`)
that computes `isGuarded = isDirty && !isSubmitSuccessful`, manages a
`beforeunload` listener, and re-arms the guard when `submitError`
appears after a successful sync submit.
- Adds `UnsavedChangesBlocker` internal sub-component (only rendered
when inside a Data Router) that calls `useBlocker` from react-router-dom
v7 and shows a MUI Dialog with **Stay** / **Discard changes** actions.
- Every consumer of `SchemaFormRenderer` (checksums, inventory,
snippets, dipper, atw, alert-troubleshooting) inherits the behaviour
automatically — no per-plugin opt-in required.

## Test plan

- [ ] Unit tests: `hooks/useUnsavedChangesGuard.test.tsx` — predicate
logic, beforeunload lifecycle, re-arm on first and consecutive failures
(AC #2, #5, #6)
- [ ] Integration tests: `SchemaFormRenderer.test.tsx` (new `describe`
block) — dialog appears on dirty navigate, Stay cancels, Discard
proceeds, clean form unblocked, post-submit navigate unblocked, re-arm
after async mutation failure (AC #1, #3, #4, #5)
- [ ] Storybook: `UnsavedChangesGuard` story — type in the field, click
"Leave this page →", confirm dialog appears; click Stay / Discard to
verify both paths
- [ ] Manual: open a plugin create/edit form, type a value, click a
sidebar link → expect dialog; submit successfully → expect no dialog on
redirect

## Notes

- `useBlocker` requires a Data Router context and will throw otherwise.
The guard is wrapped in a sub-component that's conditionally rendered
only when `UNSAFE_DataRouterContext` is present, so legacy-router and
non-router test mounts are unaffected.
- The re-arm effect has `isSubmitSuccessful` in its dep array (not the
error string), so it fires on every submit cycle regardless of whether
the error message changes — fixes a bug where consecutive failures with
the same error string would leave the guard permanently disarmed.
yyyyyyyan added a commit that referenced this pull request May 19, 2026
… QA day (#760)

## Summary

Split the `Release` GitHub Actions workflow into two dispatches so the
GAS team can validate a SHA-tagged internal-registry image during a
1-day internal QA window before `rc1` publishes to Docker Hub.

- **New `release_type: prep`** (Day 27) — `cmd_prep` + `make
release-prep` create `release/vX.Y.Z` from main HEAD, fire the rule #16
Jira version-create webhook, open the `main → vX.Y+1.0.dev0` dev-bump
PR, build the wheel, and trigger Jenkins with `pushImageDocker=false`
(SHA-tagged internal-registry image only). No version bump, no tag.
- **`cmd_rc(RC=1)` made idempotent against a prior prep** — probes
`origin` for the existing release branch and switches into an after-prep
path: syncs non-destructively to `origin/{branch}` (refuses to reset
when local is ahead), re-fires the rule #16 webhook (rule is naturally
idempotent), and gates the dev-bump PR call on `origin/main`'s actual
`pyproject.toml` version. The fresh-from-main path is bit-for-bit
unchanged (AC #6).
- **`make trigger-jenkins` parameterised** with `PUSH_IMAGE_DOCKER`
(default `true`). Deviation from the AC's literal wording
(`trigger-jenkins-internal SHA=<sha>` or `MODE=internal`): chose this
variable-on-existing-target approach as a finer-grained third path.
Pre-approved in the implementation plan.
- **Workflow YAML** adds `prep` to the choices, validates `rc_number`
must be empty for prep, and dispatches to `make release-prep`.
- 22 new unit tests cover `cmd_prep` happy path + preconditions +
ordering + head-sha → Jenkins, `cmd_rc` after-prep idempotency (webhook
re-fire, dev-bump skip-when-main-bumped, error-when-PR-unmerged,
non-destructive sync, refuse-when-local-ahead,
abort-on-rev-list-failure, still-bumps-and-tags),
`_remote_branch_exists` exit-code switch (0 / 2 / 128), and `prep`
argparse plumbing.

The workflow doc (release-process.md) is documented separately and
synced to Notion — the file is gitignored locally and does not ship in
this PR.

## Tested

- [ ] N/A — pure CI tooling. Tested via `pytest
tests/scripts/test_release.py -v` (57 tests pass), `make -n release-prep
VERSION=0.99.0`, `make -n trigger-jenkins TAG=abc
PUSH_IMAGE_DOCKER=false`, and `python3 scripts/release.py --help`. The
real integration test is the next release cycle's Day 27 dispatch by the
release manager.

## Checklist

- [x] New/modified functions have type hints and rST docstrings
- [x] New tests added for new features or bug fixes
- [x] ~~Database migrations generated if models changed (`make
makemigrations`)~~ *(N/A — no model changes)*
- [x] ~~User-facing changes documented (README, inline help, UI text)~~
*(N/A — release process change, documented in the workflow doc)*
- [x] Configuration changes documented with examples
yyyyyyyan added a commit that referenced this pull request Jun 24, 2026
Migrates the `snippets` plugin from a hand-wired JSON API to a framework
`TaskExecutionApp(script_source=ScriptSource(...))` — the first real
adoption of
the `ScriptSource` seam. The snippets engine (`app/sep/snippets/`) and
the DB
schema are unchanged; all impedance is bridged in the plugin layer.

- **`script_source.py` (new)** — a frozen `SnippetScript` adapter over
`Snippet`
plus the six hooks (`load_script`, `list_scripts`, `build_form_schema`,
`build_execution_meta`, `list_response`, `static_schema`). `load_script`
/
`list_scripts` open their own request-less session (the seam hooks take
no
request/DI) and return detached rows; `build_snippet_source` builds the
artifact
URL without a request. `SnippetScript.get_execution_model()` returns an
args-only twin of the engine model (executor host made optional) so the
framework's `model_validate(body.args)` succeeds without the
`-hostname-` field.
- **`app.py` (new)** — the `TaskExecutionApp` definition. The derived
surface is
  `GET /` (list), `GET /snippet/schema`, `GET /snippet/history`,
`POST /snippet/execute`, `GET /schema`; `GET /capabilities` is wired via
the
framework `capabilities_provider`; the legacy Jinja UI via
`jinja_router`.
- **`extra_routes.py` (new, moved from `api_routes.py`)** — the
non-derived
surface kept verbatim: `PUT`/`DELETE /snippet/approval`, `PATCH
/approvals`,
`POST /refresh`, `GET /snippet/preview`, `GET /snippet/download`.
Handler names
  are preserved so their OpenAPI operation IDs are unchanged.
- **`models.py`** — relocates `build_snippet_response` here (consumed by
  `alert_troubleshooting`) and drops the now-dead
  `SnippetExecutionRequest`/`SnippetExecutionResponse`.
- **Framework** — adds `ScriptSource.list_response_model` (additive,
defaults
`None`) and types the derived `GET /` from it, so the list endpoint
keeps its
typed `list[SnippetResponse]` response instead of degrading to untyped.
- **`settings.yaml`** — snippets is now registered through the minimal
`MODULE_NAME` entry; the exported `app` definition carries its own
metadata.
- **`__init__.py`** exports `app`; the snippets OpenAPI golden is
regenerated.

### Design decisions (accepted as part of this migration, beyond the
ticket's original out-of-scope)

1. **Framework change — `ScriptSource.list_response_model`.** Without it
the
   derived list route registers `response_model=None`, dropping the
`SnippetResponse[]` typing for snippets and every future script app. The
field
is additive and back-compatible; it is the correct spine fix rather than
a
   snippets-only knob.
2. **OpenAPI is NOT a byte-identical no-op (AC #6 relaxed).** Three
per-script
   operation IDs adopt the framework convention
(`_per_snippet_schema`→`_script_schema`, `_history`→`_script_history`,
`_execute`→`_script_execute`) and the execute request/response unify on
the
framework's `ScriptExecuteWrite` / `ScriptExecutionResponse`. The
regenerated
golden diff is limited to these plus one cosmetic description-text
change; the
request/response wire shapes are unchanged. `GET /`, `GET /schema`, and
all five
extra routes stay byte-identical, as does the `GET /schema` schema
snapshot. The
only other golden change is the `GET /capabilities` route description
dropping a
redundant `:rtype:` directive (the provider moved into `app.py` and was
cleaned
up to the docstring standard) — description text only, no path /
operation ID /
response change. AC #6's premise — that `name="snippets"` reproduces the
existing
operation IDs — does not hold against the current framework, so AC #6 is
left
   unchecked.
3. **`POST /snippet/execute` requires a configured base URL.** The
framework's
`build_execution_meta` hook is request-less, so the artifact-download
URL is
read from `SNIPPETS_BASE_URL` or `BASE_URL` (no request-derived
fallback). A
deployment with neither set now fails loudly with a 400 instead of
emitting an
   executor-unreachable localhost URL. (Changelog fragment added under
   Configuration Changes.)

### Net lines-of-code delta (AC #9)

Production `app/`: **+131** (398 added, 267 removed). Total incl. tests:
**+375**.
The first-adopter adapter (`script_source.py`, `app.py`) outweighs the
deleted
derived handlers; later script-app migrations reuse the same seam.

### Known limitations / follow-ups

- **Frontend OpenAPI client not regenerated — tracked by SEP-1386.** The
committed
`frontend/packages/api/src/generated/sep.ts` is broadly stale (it
predates the
codegen's per-app split and the resolved `openapi-typescript` 7.13.0 vs
pinned
`^7.9.0`), so regenerating produces a large diff unrelated to this
change.
  SEP-1386 owns the consolidated regen of all four clients plus a CI
codegen-freshness guard; once this PR merges, its regen picks up the
snippets
  operation-ID/model delta from the live backend.
### Bundled fix (not required by the AC)

- **The JSON-API optional-sudo toggle is now honored.** The execute form
names the
sudo field `sudo`, but the dynamic execution model validates it on the
`-sudo-`
alias with no `populate_by_name`, so `model_validate` ignored the input
and
  applied `sudo_default` — a pre-existing bug (the legacy route's
`raw_args.setdefault("sudo", body.sudo)` was a no-op for the same
reason). This
was originally going to be a byte-identical no-op, but per maintainer
request the
migration now re-attaches the explicit `ScriptExecuteWrite.sudo` choice
by
attribute name in `build_execution_meta`, so the interpreter gains its
`sudo`
prefix only when the caller opted in
(`test_optional_sudo_toggle_is_honored`).
This is the one intentional behavioral divergence from the legacy
execute path;
  non-optional snippets (NEVER/ALWAYS) are unchanged.

### Contract notes (consequences of the framework adoption)

- **Settings `api_router_path` override is no longer consulted for
snippets.** Now
that snippets exports a definition, the registry takes the definition
path and
ignores the legacy `Plugin.api_router_path` override (the same model
every
migrated `TaskExecutionApp` follows, e.g. `mysql_backups.restore`). The
whole
  app can still be disabled via the `enabled` setting.
- **`POST /snippet/execute` returns 422 (not 403) for a non-executable
snippet
submitted with invalid per-script args.** The framework validates
`body.args`
before the executability guard in `build_execution_meta` runs, so an
unapproved
snippet with malformed args surfaces the arg-validation 422 ahead of the
403.
The happy path is unchanged (non-executable + valid args still returns
403), and
there is no information leak — the per-snippet form schema is already
public via
`GET /snippet/schema` for unapproved snippets. Preserving the exact
403-first
  ordering would require a framework change and is left out of scope.
yyyyyyyan added a commit that referenced this pull request Jun 24, 2026
Migrates the `alert_troubleshooting` plugin off the registry's
legacy-synthesis fallback to an explicit declarative app, the same shape
as the `dipper` migration (SEP-1433).

- Adds `app/sep/plugins/alert_troubleshooting/app.py` exporting `app =
BaseApp(...)`, carrying the existing JSON `api_router` (`api_routes.py`)
and the deprecated Jinja router (`routes.py`) verbatim. A `BaseApp`
derives zero routes, so the JSON surface
(`/api/plugins/alert_troubleshooting/`) and the Jinja surface
(`/alert-troubleshooting/`) are unchanged.
- `__init__.py` now exports `app` alongside the preserved legacy
`router` re-export.
- `tests/app/sep/plugins/framework/test_registry.py` adds
`alert_troubleshooting` to the bespoke-`BaseApp` parametrized lists
(router-binding-by-identity, schemaless, legacy-reexport coverage).

The registry binds the explicit definition (`_bind_definition`) instead
of synthesizing one. The app's metadata
(`name`/`display_name`/`uri_path`/`css_class`/`group=alerts`/`nav_order=5`)
reproduces the existing `settings.yaml` entry, and no `schema=` is
passed (the `/schema` route is already served by the api_router's
`schema_endpoint`, so `app_schema` stays `None`). The result is a
byte-identical no-op on the `alert_troubleshooting` surface: the OpenAPI
and plugin-schema golden snapshots compare equal with no regeneration.

### Approach deviation from the ACs (mirrors SEP-1433)

The ticket's ACs describe a `TaskExecutionApp(script_source=...)`. That
mechanism is incompatible with the byte-identical-OpenAPI no-op
requirement: the framework's `ScriptSource` seam derives a fixed
filename-keyed route set (`POST /snippet/execute`, `GET
/snippet/schema`, `GET /snippet/history`) plus a flat `GET /`, with no
route-suppression hook, and never emits the composite-key `GET
/{service_type}/{alert_name}` detail this plugin's React frontend
consumes. A plain `BaseApp` derives zero routes and mounts the
hand-written routers verbatim, so it reproduces the exact surface. This
is the same resolution applied to the `dipper` migration (SEP-1433).

AC disposition:

- **Met:** alert→snippet frontmatter linking unchanged (#3); OpenAPI
byte-identical no-op (#5); golden schema byte-stable (#6).
- **Substance met via the hand-written routers, mechanism intentionally
differs from `script_source`:** package exports an `app` object but
keeps `api_routes.py` (#1); grouped list + composite-key detail
preserved exactly (#2); execution still reuses the snippets execution
dependencies via the existing Jinja `POST /execute` path (#4).
- **N/A under `BaseApp`:** no `script_source` adapter and no adapter
unit test (#7); the `TaskExecutionApp`-only structural conformance
checks skip for a `BaseApp`, while the registry-wide route-collision
check covers the new app (#8).

Changelog: N/A — the migration is a purely internal
registration-mechanism change with no user-visible effect (mirrors
SEP-1433, which added no fragment).

## Bundled change: frontend OpenAPI spec regeneration (absorbed by
`main`)

This branch carried a `frontend/packages/api/specs/sep.json`
regeneration — a consequence of #993 (`ci: fix dump_openapi.py script`),
which fixed the spec generator but left the committed spec stale. `main`
has since merged its own regeneration (`fix: regenerate spec`), so after
merging current `main` into this branch the spec is byte-identical to
`main` and no longer appears in this PR's net diff. It changes **no**
`alert_troubleshooting` paths. The net diff is now just the three
migration/test files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant