cli: Add pipeline copy command#6509
Conversation
Closes feldera#6399 Signed-off-by: Dnreikronos <dnreikronos2@gmail.com>
| .unwrap() | ||
| .into_inner(); | ||
|
|
||
| let Some(program_code) = source_pipeline.program_code else { |
There was a problem hiding this comment.
not for this PR, but I wonder how this can even be None, it should always have a value (maybe cc @snkas knows)
There was a problem hiding this comment.
yeah good point. afaict this should not be none here.
for this call we don't pass a selector, and the api default selector is all, so program_code should come back on a current server. same for runtime_config and program_config.
i kept the guards bc the generated client type has these as option, since selected pipeline responses can omit fields when a narrower selector is used. but imo for fda copy this is basically unreachable unless the server is old or returning a weird response.
ltm maybe the better move is to treat this as an invariant instead of showing the upgrade msg, since that msg makes it look like the user can fix it. lgtm to change if you prefer.
| ); | ||
| std::process::exit(1); | ||
| }; | ||
| let Some(runtime_config) = source_pipeline.runtime_config else { |
| ); | ||
| std::process::exit(1); | ||
| }; | ||
| let Some(program_config) = source_pipeline.program_config else { |
gz
left a comment
There was a problem hiding this comment.
makes sense do you mind also updating test.bash to exercise the command (I think we have an "implicit" copy in there somewhere there maybe we can use this command)
mythical-fred
left a comment
There was a problem hiding this comment.
Approve. Tight, obvious win — manually cloning a pipeline by stitching together program set + set-config + tag commands is exactly the kind of paper-cut that belongs as a built-in. The clone alias is a nice touch.
Test covers both names. Diff is small and reads cleanly.
A few non-blocking notes:
-
crates/fda/src/main.rs— the threelet Some(...) = source_pipeline.X else { exit 1 }blocks forprogram_code,runtime_config,program_config. gz already flagged that these are effectively unreachable on a current server; I agree the defensive guards are the right call for cross-version safety, but the error text could distinguish "this is a server-side bug" from "you're talking to an old server" — right now both look identical, and only one is actionable. Either drop theUPGRADE_NOTICEsuffix for fields that should always be populated, or add a hint that the user should file a bug. -
The new pipeline inherits
descriptionandtagsverbatim. That's the most-useful default for the "variant for testing" workflow in #6399, but it does meanfda copy prod prod-testproduces two pipelines with identical descriptions like "Production order ingestion". Worth a single sentence in--help(or the long help block) saying the description is copied as-is, so users don't get surprised. Not a blocker — easy to follow up withfda set description. -
Output format: text branch prints a one-liner success message; JSON branch prints the full response. The
_ => Unsupported output formatarm is consistent with the rest of the CLI but worth noting that any future format (e.g. yaml) added globally will need an update here. Existing pattern, not your problem. -
Minor: the
Copydoc-string says "Copy a pipeline's program and configuration into a new pipeline." Worth mentioning UDFs and tags are also copied, since the issue explicitly asks for that.
Closes #6399
idk, manually cloning a pipeline by stitching together program and config commands feels easy to mess up. imo this belongs as a tiny CLI shortcut, since testing variants should not make you copy the same fields by hand.
Adds
fda copy SOURCE DESTINATION, pluscloneas an alias. It creates the new pipeline from the source pipeline's program, UDFs, runtime config, compilation config, description, and tags. ltm this is better as a boring built-in command than a shell recipe. lgtm for the testing workflow in the issue.