Skip to content

pipeline-manager: require program_info_url and schema unit test#6294

Merged
ryzhyk merged 1 commit into
manager-ignore-schemafrom
manager-ignore-schema-edits
May 21, 2026
Merged

pipeline-manager: require program_info_url and schema unit test#6294
ryzhyk merged 1 commit into
manager-ignore-schemafrom
manager-ignore-schema-edits

Conversation

@snkas
Copy link
Copy Markdown
Contributor

@snkas snkas commented May 21, 2026

The program_info_url is now a mandatory parameter for a pipeline to be able to be started, as its absence has been long enough deprecated. An error is added in case it is missing. This makes it consistent how program info is handled across runners.

Add a unit test for the schema with only properties.

The `program_info_url` is now a mandatory parameter for a pipeline to be
able to be started, as its absence has been long enough deprecated. An
error is added in case it is missing. This makes it consistent how
program info is handled across runners.

Add a unit test for the schema with only properties.

Signed-off-by: Simon Kassing <simon.kassing@feldera.com>
Copy link
Copy Markdown

@mythical-fred mythical-fred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Clean follow-up to #6288.

The new ProgramSchemaPropertiesOnly unit test directly answers the schema subset-parsing concern I raised on #6288 — good. The Option<&str> -> &str plumbing in PipelineExecutor::provision plus the hard AutomatonCannotConstructProgramInfoUrl transition is the right cleanup; there is only one real implementor (LocalRunner) plus the mock and both are updated.

One soft note (non-blocking):

  • The new hard-error transit path in pipeline_automata.rs — when program_info_integrity_checksum is None — has no test in runner::pipeline_automata::test. The missing_artifacts_sets_pending scaffolding is already there; a tiny test that drives a pipeline whose program_info_integrity_checksum was never recorded into TransitionToStopping{ AutomatonCannotConstructProgramInfoUrl } would lock the new contract in place. Worth a tracking issue at minimum, since this is the exact "long-enough-deprecated path now hard-errors" behavior change called out in the PR description.

Copy link
Copy Markdown
Contributor

@ryzhyk ryzhyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thanks!

@ryzhyk ryzhyk merged commit 65a8830 into manager-ignore-schema May 21, 2026
1 check passed
@ryzhyk ryzhyk deleted the manager-ignore-schema-edits branch May 21, 2026 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants