Conversation
|
Promptless prepared a documentation update related to this change. Triggered by runpod/flash#320 Documented the new source fingerprinting feature that enables Flash to automatically trigger rolling releases when code changes without config changes. Updated deployment docs, build process documentation, and app deployment guide. |
There was a problem hiding this comment.
Pull request overview
This PR adds a “source fingerprint” mechanism so code-only changes (with no resource config changes) still flow through the existing config-diff reconciliation path and trigger rolling releases.
Changes:
- Add
compute_source_fingerprint()to compute a SHA-256 digest over the user’s source file set and persist it intoflash_manifest.jsonduringflash build. - Inject
_FLASH_SOURCE_FINGERPRINTinto each resource’senvin reconciliation so fingerprint drift shows up as a config diff. - Add unit tests covering fingerprint computation and reconciliation behavior for matching/mismatching/missing fingerprint cases.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/runpod_flash/cli/commands/build.py |
Adds fingerprint computation and writes it into the manifest during build. |
src/runpod_flash/cli/utils/deployment.py |
Injects fingerprint into resource env prior to config comparison to drive update decisions. |
tests/unit/cli/commands/test_build.py |
Adds unit tests for fingerprint determinism and sensitivity properties. |
tests/unit/cli/utils/test_deployment.py |
Adds reconciliation tests for fingerprint-driven update/reuse/back-compat flows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Inject source fingerprint into each resource's env so that code-only | ||
| # changes (no resource config diff) still trigger a rolling release. | ||
| # The fingerprint is computed during flash build from user source files. | ||
| source_fingerprint = local_manifest.get("source_fingerprint") | ||
| if source_fingerprint: | ||
| for resource_config in local_manifest.get("resources", {}).values(): | ||
| env = resource_config.setdefault("env", {}) | ||
| env["_FLASH_SOURCE_FINGERPRINT"] = source_fingerprint |
There was a problem hiding this comment.
Injecting _FLASH_SOURCE_FINGERPRINT into the local resource env will force local_json != state_json for any existing resource whose stored state config has no env/no fingerprint (e.g., deployments created before this feature). That effectively triggers a rolling update on the first deploy after upgrading, even if code and user config are unchanged. If that’s not intended, consider treating a missing fingerprint in state_config as "unknown" (don’t update), but still persist the injected fingerprint back to state/local manifest so subsequent deploys can detect real code-only changes.
There was a problem hiding this comment.
Acknowledged. This is a one-time migration cost: the first deploy after upgrading flash will trigger a rolling release on every endpoint that was deployed before this feature existed. After that, the state manifest has the fingerprint and normal behavior resumes.
Accepting this rather than adding a fallback because:
- The rolling release is harmless — workers are recreated with the same image, scaling, env vars, etc.
- Users upgrading flash generally expect some churn on the first post-upgrade deploy.
- Adding a "treat missing state fingerprint as unknown" branch adds complexity for a one-time event.
Leaving this thread open for the reviewer to confirm before merge.
runpod-Henrik
left a comment
There was a problem hiding this comment.
What it does
Computes a SHA-256 fingerprint of user source files at build time, writes it into flash_manifest.json, then injects _FLASH_SOURCE_FINGERPRINT into each resource's env during reconciliation. Since env is included in the JSON config comparison (_resource_config_for_compare), a code-only change produces a diff and routes to the update path. Zero changes to ServerlessResource, ResourceManager, or the update machinery.
The design is sound. The key invariants hold:
.flash/is excluded byload_ignore_patterns, so the previous build'sflash_manifest.jsonis not included in the fingerprint (no circular dependency)envis not inRUNTIME_RESOURCE_FIELDS, so the injected value reaches the JSON comparisonsetdefault("env", {})preserves existing user-defined env vars
Findings
1. import hashlib is inside the function body
build.py, compute_source_fingerprint
def compute_source_fingerprint(...) -> str:
import hashlib # ← should be at module top
h = hashlib.sha256()Python caches imports so it's not a performance issue, but it's inconsistent with every other import in the file and makes static analysis tools skip it.
2. local_manifest is mutated in place — undocumented side effect
deployment.py:288–292
for resource_config in local_manifest.get("resources", {}).values():
env = resource_config.setdefault("env", {})
env["_FLASH_SOURCE_FINGERPRINT"] = source_fingerprintThis mutates the caller's dict before app.update_build_manifest runs, which is actually the correct mechanism for persisting the fingerprint into state. But nothing in the code says so. A one-line comment (# mutation intentional: persisted to state manifest below) would prevent a future reader from "fixing" it by taking a copy.
3. Test asserts call count, not the injection value
test_deployment.py, test_source_fingerprint_injected_into_resource_env
The test proves the update path fires for both resources but doesn't verify the actual env contents:
assert mock_manager.get_or_deploy_resource.call_count == 2
# missing: assert local_manifest["resources"]["worker"]["env"]["_FLASH_SOURCE_FINGERPRINT"] == "abc123def456"Also, lb_endpoint already has USER_VAR: value in its env. The test doesn't assert that key is preserved after injection. Worth adding given the setdefault path.
4. _FLASH_SOURCE_FINGERPRINT is visible in worker environment
Low severity, but worth knowing: since the env dict is passed through to the deployed resource, every worker will have _FLASH_SOURCE_FINGERPRINT=<hex> in its runtime environment. Users inspecting env vars or logging os.environ will see it. The underscore prefix signals internal, but there's no documentation of this var anywhere. If Flash ever documents its reserved env vars, this should be on the list.
5. Any file tracked by .gitignore affects the fingerprint — including non-code assets
compute_source_fingerprint hashes all files returned by get_file_tree, not just .py files. Renaming a README, adding a data/ CSV, or changing a config JSON will trigger a rolling release. This is probably the right call (any artifact change should redeploy), but it's worth noting in the PR so users aren't surprised when a doc-only edit triggers a deploy.
Summary
| Core mechanism | Correct |
| Backward compat | Correct (no fingerprint → falls through to config-only comparison) |
| Fingerprint scope | Correct (.flash/ excluded, no circular dep) |
| Env preservation | Correct (setdefault + assign doesn't clobber existing keys) |
| Blocking issues | None |
| Should-fix before merge | #1 (trivial), #3 (add assertion) |
| Should-document | #2 (comment), #4 (reserved vars list) |
Computes a SHA-256 fingerprint of user source files during build. Deterministic, content-sensitive, path-sensitive, and order-independent. Used to detect code-only changes that should trigger rolling releases.
Calls compute_source_fingerprint after manifest builder produces the manifest dict, injecting the hex digest before writing to disk.
…reconciliation Code-only changes (no resource config diff) previously took the reuse path and never triggered a rolling release. Injecting the source fingerprint computed at build time into each resource's env field means a code change produces a diff in the comparison JSON, routing to the update path and triggering a rolling release. - Inject _FLASH_SOURCE_FINGERPRINT into resource env before comparison - No-op when source_fingerprint absent (backward compatibility) - Three tests: fingerprint changed triggers update, unchanged takes reuse path, missing fingerprint falls back to config-only comparison
- Move hashlib import to module level (consistency with file conventions) - Add comment clarifying intentional local_manifest mutation in reconciliation - Assert injected fingerprint value and user env var preservation in test
d75a0d8 to
193e4c4
Compare
- Normalize paths to POSIX form so Windows and Unix builds of the same project produce the same fingerprint - Add length-prefix framing to fingerprint hash input to eliminate theoretical path/content concatenation ambiguity - Restructure test so the fingerprint value is the only diff between local and state manifests, proving the injection drives the update
| # Inject source fingerprint into each resource's env so that code-only | ||
| # changes (no resource config diff) still trigger a rolling release. | ||
| # The fingerprint is computed during flash build from user source files. | ||
| source_fingerprint = local_manifest.get("source_fingerprint") | ||
| if source_fingerprint: | ||
| for resource_config in local_manifest.get("resources", {}).values(): | ||
| env = resource_config.setdefault("env", {}) | ||
| env["_FLASH_SOURCE_FINGERPRINT"] = source_fingerprint |
Summary
Subsequent
flash deploywith code changes but no resource config changes now triggers a rolling release.flash build, write into manifest_FLASH_SOURCE_FINGERPRINTinto resource env before config comparison during reconciliationWhat changed
cli/commands/build.pycompute_source_fingerprint()+ call inrun_build()cli/utils/deployment.pyZero changes to
ServerlessResource,ResourceManager, orupdate().Test plan
compute_source_fingerprint(deterministic, content-sensitive, path-sensitive, order-independent, empty input, binary files)source_fingerprint(older flash) behaves as before