CLI: Deploy straight from yaml - #1481
Closed
josephjclark wants to merge 13 commits into
Closed
Conversation
It's wrong, the mock must not change
Several things wrong
* Worker: better reporting on socket errors (#1505) * Worker: attribute sentry reports to the run that produced them Each run already opened a sentry isolation scope, but engine and socket callbacks fire outside its async context, so breadcrumbs and errors were landing on the global scope and interleaving across concurrent runs. The scope is now held on the run context and re-entered where the reporting actually happens. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * carry payload size on step-complete events for better reporting * type fix * add payload size to step complete event * report payload size to sentry * changeset --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * Worker: Fix dataclip serialization (#1507) * Worker: attribute sentry reports to the run that produced them Each run already opened a sentry isolation scope, but engine and socket callbacks fire outside its async context, so breadcrumbs and errors were landing on the global scope and interleaving across concurrent runs. The scope is now held on the run context and re-entered where the reporting actually happens. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * carry payload size on step-complete events for better reporting * type fix * report better errors out of sentry * Gate double-encoding fix for output dataclips behind WORKER_NO_STRINGIFY_STATE The worker double-JSON-encodes step output dataclips before sending them to Lightning: once via a manual stringify, then again when phoenix serializes the envelope. That double-encoding is what bloats large dataclips past Lightning's websocket frame limit and kills the connection mid-run. The runtime already sanitizes/clones state before it reaches this point, so the manual stringify is redundant - but skipping it changes the wire format, so it's opt-in via --stringify-state/--no-stringify-state (CLI) or WORKER_NO_STRINGIFY_STATE (env), default false to preserve current behaviour until the matching Lightning-side support (2.19+) is widely deployed. * docs and changelog --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * versions --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Fix redaction so oversized state is actually replaced, not silently left intact * display warning message when final_state is redacted * changeset * worker@1.29.3
* Carry a webhook's custom path through deploy OpenFn/lightning#5105 lets a webhook trigger be named, so its URL is known before it is deployed. Deploy never sent the field. The new-trigger path builds its payload from an allowlist that does not include it, and the existing-trigger path builds its fields explicitly. A path only travels when the spec mentions it. An absent key means the server keeps what it has, so deploying a spec written before this existed does not wipe a path set through the app. Writing it blank clears it, either as an empty string or as a bare key, which YAML reads as null. Cron and kafka triggers ignore it, matching the server. Both insertion points decide on the resolved trigger type rather than on what the spec wrote, because type is optional in a spec and defaults to webhook. A trigger that names a path and nothing else is a legal spec, and guarding on the spec's own type would drop the path without saying so. Pull needs no change. It fetches the yaml from Lightning, which now exports the field. * Keep a webhook's custom path top-level when reading app state `from-app-state` destructures the trigger keys it knows and sweeps the rest into `openfn:`, so a pulled `custom_path` landed nested there rather than beside `webhook_reply`. The v1 and v2 project formats would have disagreed about where a webhook's path lives. The workflow version hash is deliberately left alone. Lightning's `canonical_form/1` does not hash `custom_path` either, and adding it on one side only would make the two hashes disagree for every workflow holding a path, which is what `find-changed-workflows` compares. Moving it needs both sides at once. * Hash the custom path, and stop deploy reporting removals it will not make Two gaps the review left open. The workflow version hash ignored custom_path, so find-changed-workflows saw no change when a webhook was renamed and openfn project deploy skipped it. Lightning hashes the same key now, in OpenFn/lightning#5105, and the parity test against a real Lightning hash still passes: both sides leave a workflow without a path hashing exactly as it did. And deploy diffs the server's project against the payload it is about to send, so every trigger field held on the server and never named in the spec showed up as a removal that was not going to happen. Absence means leave it alone, so the diff now hides what the payload does not carry. webhook_reply and webhook_response_config had this already. * Show what a deploy will clear, and hash a path only where it means something Three things from review. The diff mask hid fields the server was about to wipe. validate_by_type clears a path when a webhook becomes a cron, and clears the response config when the reply mode stops being after_completion, whether or not the payload names them. Masking those made the user confirm a deploy that retires a live URL with nothing in the diff saying so. The mask now stands down when the type or the reply mode is changing, and for a trigger being deleted, where the diff is the only place the user sees what goes with it. A path on a cron or kafka trigger is no longer hashed. It never served a URL, and Lightning stopped hashing one there in the paired PR, so the two would otherwise disagree. And the type check: the fixture's inferred literal type has no custom_path, so the new test needed the cast the rest of that file already uses. ava does not type check, and I ran it instead of tsc. * Narrow the spec trigger instead of asserting it Review point. The non-null assertion says the value is there without anything enforcing it. Nothing can reach this line with an undefined spec trigger today, since splitZip walks the union of keys and the two branches above return for spec-only and state-only, but the check costs nothing and does not depend on that staying true. * One changeset per package Deploy had two, a minor for the feature and a patch for the diff fix. The tool flattens them to the higher bump anyway, and one entry per package per release is the shape everywhere else. * Bump versions deploy 0.15.0, project 0.19.0, lexicon 2.4.3, and the dependents changesets pulls along with them. * Cut the changelog entries to a sentence CLAUDE.md says a changeset note is a single short high-level sentence with no implementation detail. Mine were three paragraphs of it. * update canonical test --------- Co-authored-by: Joe Clark <jclark@openfn.org>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Collaborator
Author
|
Right something I need to think about here if you post a yaml directly, how do we know it's a new project and you don't just want to force push an update? Presumanly you should be able to manuaally edit the yaml file and force push it from the file. A regular deploy will update from the file system but why not just edit the yaml? You can pass --new to force it but eh it should be obvious right? The project is new if either
So let's add a little automation around that |
…deploy Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Collaborator
Author
|
@midigofrank just a headsup that I'm close to getting this closed out - expecting to release tomorrow |
Collaborator
Author
|
Rebuilt as #1533 because the base got messed up (idk even what's happened here) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Short Description
This PR enables users to run a deploy from a file: either an exported project.yaml (from the app) or from A single v2 state file (the thing created when fetching a project).
Note that if you try and deploy a project to the same instance and the same UUID, it'll just override. In this case you have to pass
--newor set a new name.A side-effect of this work is that you can now edit a local project.yaml file and deploy it directly to lightning - previously the CLI didn't allow this because a deploy would re-build the project.yaml from the local filesystem. Being able to modify the yaml directly is probably a useful escape hatch. But one I'll leave undocumented for now.
Fixes #1264
QA Notes
To test this ( I recommend running against local or at least staging)
You can set
OPENFN_ENDPOINTandOPENFN_API_KEYenv vars to configure deploy targets and access and stuff..envfiles work.Scenario 1 - deploy an exported project
openfn project deploy downlaoded-project.yaml --name my-duplicateScenario 2 - re-deploy an existing project
openfn project pull <uuid> --alias ogog@app.openfn.org.yamlopenfn project deploy ./projects/og@app.openfn.org.yaml --name my-duplicateAI Usage
Please disclose whether you've used AI anywhere in this PR (it's cool, we just
want to know!):
You can read more details in our
Responsible AI Policy