6ecb2c366f
Publish snapshot / snapshot (push) Successful in 1m2s
[fix] default parameter run records parameters in session for replay[fix] remove default parameters for several cubes
7.7 KiB
7.7 KiB
Nopy Refactoring Plan
This document tracks the major refactoring of the nopy package.
Refactoring Items
1. Remove parallel execution
- Status: ✅ Completed
- Goal: Remove all logic supporting parallel execution of cubes to simplify the execution flow and improve reliability.
- Rationale: The feature never shipped. Concurrent pyinfra processes interleave their output, which made deployment logs unreadable — a cost that outweighed the wall-clock saving. Do not reintroduce it without first solving per-cube output buffering.
- Context:
- Parallelism removed from
NopyConfig,NopyOptions, andexecuteDeployCalls. buildExecutionStagesdeleted.- CLI flags
--paralleland--concurrencyremoved. - Documentation caught up later:
README.md,docs/API.md, anddocs/HOOKS.mdhad all continued to describe the feature as if it existed.
- Parallelism removed from
- Proposed Solution: (Done)
2. Rework cube building process & Dependency Resolution
- Status: ✅ Completed
- Goal: Allow dependencies to be defined as a function of the collected variables.
- New Signature:
dependencies?: (variables: CubeVariables) => DependencySpec[] - Architectural Change: Implement a clean, step-based resolution mechanism using a
BuildContext. - Context:
- Introduced
BuildContextincubes/dependencies.tswhich handles recursive resolution, variable collection, and hook execution. - Resolution is now dynamic: variables are collected for a cube before its dependencies are resolved.
- Introduced
- Proposed Solution: (Done)
3. Remove env property from cube Manifest
- Status: ✅ Completed
- Goal: Remove the
envproperty from the cube manifest. - Context:
envremoved fromManifestandEnvtypes.- Responsibility for defaults shifted entirely to Zod schema defaults and
getDefaults().
- Proposed Solution: (Done)
4. Redesign Manifest and Cube types
- Status: ✅ Completed
- Goal: Transition from
Env -> Manifest -> Cubeinheritance to a cleanerManifest(specification) andCube(runtime) separation. - Context:
Manifestis now a clean interface with a factory namespace.Cubeis a class encapsulating aManifestand runtime info (dir,deployScript).
- Proposed Solution: (Done)
5. Make --use-defaults operational
- Status: ✅ Completed
- Goal: Turn
-Dfrom a flag that was parsed and threaded through three layers but never read into a working non-interactive mode. - Rationale: Unattended runs — CI, or provisioning a fresh box from a checked-in
.nopyrc.json— are the reason the flag exists. It prompted anyway. - Context:
BuildContext.resolveCubebranches onoptions.useDefaultsand skipsVariableAssignment.Variables.get()merge order corrected to defaults → globalenv→ prompts → params.envused to lose to the schema default, which left a non-interactive run with no way to be configured at all.- Replayed session values moved from the
defaultsscope toprompts, so they keep outrankingenvnow thatenvsits higher. Cube.getDefaults()no longer discards every default when one field lacks.default(); it falls back to a per-field read.Cube.requiredKeys()added, and a-Drun fails naming the unfillable variables instead of deploying a cube with them absent from--data.VariableAssignmentoffers every schema key, not only the ones carrying a default, and shows the value the run would actually use as the initial.
- Proposed Solution: (Done)
6. Make variable assignment a first-class concept
- Status: ✅ Completed
- Goal: Give a variable an identity and a provenance, instead of inferring both from which bag it happened to sit in.
- Rationale: Item 5 left precedence encoded as the field order of an object literal inside
Variables.get()—defaults, thenglobal, thenprompts, thenparams. Nothing named the ranking, nothing could be asked where a value came from, and a replay had to be smuggled into thepromptsbag because there was no origin that meant "recorded". Every question that followed — what should a session record, which values are safe to print — needed provenance to answer. - Context:
Assignment { value, origin }and anOriginrankeddefault(0) < env(1) < session(2) < prompt(3) < param(4). Precedence is now data, not the order lines appear in.Variableis a class over an assignment list.assignmentsis the true history, newest first and never reordered;orderedis a stable sort of it by origin rank, andvalue/originread the head of that. Stability is what makes the two views coexist: same-origin ties keep the newest in front while the value it displaced stays visible.- The
globalbag is gone. Configenvis seeded per cube as a real assignment at originenv, sovariables.get('global')— a cube id that was never a cube — is no longer a thing. - Replay assigns at origin
session, which outranksenvanddefaulton its own. Theprompts-bag workaround is deleted. - A session records
Variables.persistable()— every effective value, not just prompted ones. A-Drun used to record nothing and replay by re-deriving from whatever the defaults said at replay time.
- Trade-off accepted: recorded values now outrank the current
.nopyrc.jsonenvand the current schema defaults, so editing either no longer leaks into an existing session's replay. That is the point of a snapshot, but it does mean picking up a new default requires re-recording. - Proposed Solution: (Done)
7. Manifest-declared secrets
- Status: ✅ Completed
- Goal: Let a manifest say which schema keys hold sensitive values, and act on it.
- Rationale: Item 6 made sessions record everything, which forced the question of what must not be recorded. The codebase already had an answer of sorts —
outputExecutionPlanmasked any variable whose name contained "password" — that missedTOKEN,PSKandAUTH_KEY, and was defeated anyway by the unmasked command printed one line above it. - Context:
Manifest.secrets?: string[], validated at load: an entry that is not a key ofschemais a manifest error and aborts the run, so a typo cannot silently leave a value unprotected.- Deliberately a plain array, not zod metadata.
.meta()and.describe()store intoz.globalRegistry, which is per-copy — a manifest built by a different zod copy would look up empty. Fail-open is fine for a missing prompt label and unacceptable for a secret marker. maskCommand()replaces declared--datavalues and the SSH--passwordin the command string itself, and is wired into--print-only, the dry-run plan and the debug log. Thenopylogger runs atlowestLevel: 'debug', so that last one was printing credentials on every run.- Secrets are excluded from
persistable(), so a replay has a gap where one used to be.fillSessionGapsprompts forrequiredKeys() ∪ secrets; under-Dit fails naming them, consistent with item 5's fail-fast.
- Scope limit:
secretskeeps a value out of what nopy writes. The value is still on pyinfra's command line (visible inps), still echoed by the variable form, and a.default()is still plain text in the manifest. Documented rather than fixed — the first is inherent to pyinfra's interface. - Bug fixed along the way:
cubes/user/addgenerated a random password as its schema.default(). Because the key had a default it was never inrequiredKeys(), and because a generated default is re-evaluated on every read, an unattended run created an account with a credential nobody had seen and a replay created a different one again. It is now the literalchangeme. - Proposed Solution: (Done)