Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement pipeline validation for unsupported states #1043

Merged
merged 9 commits into from
Sep 14, 2023

Conversation

dpadhiar
Copy link
Contributor

@dpadhiar dpadhiar commented Sep 13, 2023

Partially completes #1002

Adds pipeline validation to prevent pipelines to be submitted that have:

  • the last vertex not be a sink
  • duplicate edges
  • dangling vertices

@dpadhiar dpadhiar marked this pull request as ready for review September 13, 2023 23:46
@@ -89,6 +89,17 @@ func ValidatePipeline(pl *dfv1.Pipeline) error {
return fmt.Errorf("pipeline has no sink, at least one vertex with 'sink' defined is required")
}

for _, v := range pl.Spec.Vertices {
Copy link
Member

Choose a reason for hiding this comment

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

Shall we do something like below:

  1. Sources should have to, but no from,
  2. Sinks should have from, but noto;
  3. UDFs should have both.

And this can be done together with the for loop above.

Copy link
Member

Choose a reason for hiding this comment

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

If this logic is in place, maybe we can remove the logic in line 165-170.

Copy link
Member

Choose a reason for hiding this comment

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

+1.

pkg/reconciler/pipeline/validate.go Outdated Show resolved Hide resolved
@@ -89,6 +89,17 @@ func ValidatePipeline(pl *dfv1.Pipeline) error {
return fmt.Errorf("pipeline has no sink, at least one vertex with 'sink' defined is required")
}

for _, v := range pl.Spec.Vertices {
Copy link
Member

Choose a reason for hiding this comment

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

+1.

dpadhiar and others added 8 commits September 14, 2023 12:08
Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
…PendingNotAvailable (numaproj#1040)

Signed-off-by: Keran Yang <yangkr920208@gmail.com>
Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
…ch (numaproj#1041)

Signed-off-by: Derek Wang <whynowy@gmail.com>
Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
Signed-off-by: veds-g <guptavedant2312@gmail.com>
Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
Signed-off-by: Derek Wang <whynowy@gmail.com>
Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
Co-authored-by: Keran Yang <yangkr920208@gmail.com>
Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
@dpadhiar dpadhiar merged commit 9035ba8 into numaproj:main Sep 14, 2023
17 checks passed
whynowy pushed a commit that referenced this pull request Sep 15, 2023
Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
Signed-off-by: Keran Yang <yangkr920208@gmail.com>
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.

None yet

4 participants