Pipeline inputs #4499

ukclivecox · 2022-12-12T18:52:44Z

Allows pipelines to take inputs from other pipelines.

Adds new input section to pipeline spec
Updates dataflow engine to take into consideration the pipeline to filter and pipeline header to add after inputs
Adds an example notebook
Adds docs

Does not (maybe for follow on PRs) :

check for circular dependencies
extend pipeline status to include status of dependent pipelines

review-notebook-app · 2022-12-12T18:52:48Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

… example

sakoush

Leaving some comments for now, until I go over the remaining bit.

apis/go/mlops/scheduler/scheduler_grpc.pb.go

apis/go/mlops/scheduler/storage.pb.go

apis/go/mlops/v2_dataplane/v2_dataplane_grpc.pb.go

apis/mlops/chainer/kotlin/io/seldon/mlops/chainer/BatchKt.kt

docs/source/contents/pipelines/index.md

sakoush · 2022-12-14T16:56:53Z

docs/source/contents/pipelines/index.md

+
+Further examples can be found in the [pipeline-to-pipeline examples](../examples/pipeline-to-pipeline.md).
+
+Present caveats:


sakoush

Leaving more comments, mostly questions.

sakoush · 2022-12-14T21:28:29Z

scheduler/pkg/kafka/topics.go

@@ -106,3 +106,33 @@ func (tn *TopicNamer) GetFullyQualifiedTensorMap(pipelineName string, tin map[st
 	}
 	return tout
 }
+
+func (tn *TopicNamer) GetFullyQualifiedPipelineTensorMap(tin map[string]string) map[string]string {


nit: I would personally use more representative variable names instead tin and tout

sakoush · 2022-12-14T21:29:21Z

scheduler/pkg/kafka/topics.go

+		parts := strings.Split(k, pipeline.StepNameSeperator)
+		var kout string
+		switch len(parts) {
+		case 3:


can you add comment in code why 3 and 5?

scheduler/pkg/kafka/topics.go

scheduler/pkg/proxy/chainer.go

scheduler/pkg/store/pipeline/validate.go

sakoush · 2022-12-14T22:04:56Z

scheduler/pkg/store/pipeline/utils.go

@@ -200,6 +252,29 @@ func updateInputSteps(pipelineName string, inputs []string) []string {
 	return updatedInputs
 }

+func updateExternalInputSteps(inputs []string) []string {


could we add more documentation and/or unit test to explain logic?

will add test

scheduler/pkg/kafka/dataflow/server_test.go

sakoush

In general this looks a great feature that would allow decoupled end-2-end pipeline creations.
However we should make sure that we have the right tooling to support users with their async decoupled pipelines and they are able to inspect and debug dataflow end-2-end.

So perhaps we want to discuss what would be potential follow ups beyond this PR.

apis/mlops/chainer/kotlin/io/seldon/mlops/chainer/ChainerOuterClassGrpcKt.kt

apis/mlops/chainer/kotlin/io/seldon/mlops/chainer/ChainerOuterClassKt.kt

apis/mlops/chainer/kotlin/io/seldon/mlops/chainer/PipelineStepUpdateKt.kt

apis/mlops/chainer/kotlin/io/seldon/mlops/chainer/PipelineSubscriptionRequestKt.kt

apis/mlops/chainer/kotlin/io/seldon/mlops/chainer/PipelineUpdateMessageKt.kt

scheduler/pkg/kafka/dataflow/server_test.go

scheduler/pkg/proxy/chainer.go

scheduler/pkg/store/pipeline/pipeline.go

sakoush · 2022-12-15T11:33:30Z

scheduler/pkg/store/pipeline/utils.go

+		case 1: // Add outputs if just pipeline specified
+			updatedInputs = append(updatedInputs, fmt.Sprintf("%s.%s", inp, StepOutputSpecifier))
+		case 3: // Add outputs if step name only specified


as related to previous comments, I am not sure for the reader why it is 1 or 3 parts? perhaps add more description in code.

scheduler/pkg/store/pipeline/validate.go

seldondev · 2022-12-15T11:44:43Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sakoush
To complete the pull request process, please assign
You can assign the PR to them by writing /assign in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…ensormap

Co-authored-by: Sherif Akoush <sherif.akoush@gmail.com>

* Capitalise Seldon when used as proper noun * Formatting, capitalisation, etc. * Add detail to inference docs page * Formatting and typo fixes * Use consistent capitalisation of model & pipeline through inference docs page * Fix typo in inference docs for Kafka topics * Specify use of inference v2 protocol high up in inference docs * Add mention of headers to sync inference introduction * Formatting + minor rewording for clarity * Use tabs for Compose vs. k8s methods for finding the seldon-mesh endpoint * Add note on port-forwarding seldon-mesh svc for inference requests * Add note on service meshes for sending inference requests * Add section on inference request routing with headers * Add section on path-based routing for inference endpoints * Add subsection header for Seldon routing (vs. ingress routing) * Add section on routing from ingress -> seldon-mesh for inference calls * Add links to RFCs for host & authority headers * Update link to RFC for HTTP/1 Host header RFC-7230 obsoletes RFC-2616, the previous link. * Add line describing virtual hosts vs. physical ones * Use tabs for alternate ways of making inference requests * Add inference request example with Seldon CLI * Use consistent capitalisation of v2 for inference protocol * Add note on Kafka headers for pipelines * Use ordinal numbering for bullet points * Update URI for consistency and to avoid confusion * Move section on making requests above section on routing * Use interpolation syntax to clarify usage of path-based routing in Seldon mesh * Add second form of path-based routing for pipelines in Seldon mesh * Clarify wording re virtual endpoints in SCv2 * Add section for header-based routing examples This section builds on the examples from the prior section on making inference requests. * Update basic examples to exclude routing headers Routing headers are then given in the examples relevant to that section. * Formatting * Use group-tabs for example requests with different clients * Add emphasis to header lines in examples for header-based routing * Add notes on support for subdomain-based routing * Add example snippets for subdomain routing * Add Open Inference schema for iris model for examples * Move pipeline inference tip lower for better flow * Fix datatype for iris model inputs

* add flag for autoscaling in grpc msg * autogen files * extract helper function * adjust comment * wire up autoscaling flag in server * wire up autoscaling in agent client * set thresholds for scaling in local deployment * add autoscaling flag to scheduler * add a toggle for autoscaling service * revert autoscaling envs set in local deployment * disable scaling for local deployment * use a disable toggle instead * do not disable by default scaling service

Not sure if this is necessary but it actually took me some time to figure it out as I was sure that I have `docker compose` already installed. According to the [Docker documentation](https://docs.docker.com/compose/reference/) the spaced version looks like the newer one and maybe the makefile should be updated for that.

* Fix possible SIGSEV after producer close in modelgateway * Set running after setup * review comments

* link compose github for easier installation * Update docs/source/contents/getting-started/docker-installation/index.md Co-authored-by: Alex Rakowski <20504869+agrski@users.noreply.github.com> Co-authored-by: Alex Rakowski <20504869+agrski@users.noreply.github.com>

sakoush

LGTM

ukclivecox added 3 commits December 11, 2022 19:08

initial commit for pipeline inputs from other pipelines

c1cdcdc

Add extended example

b140009

add trigger example

949f9a5

seldondev added the size/XXL label Dec 12, 2022

ukclivecox requested a review from sakoush December 12, 2022 18:53

ukclivecox added 2 commits December 12, 2022 19:14

fix kafka header test - headers now lowercase

b5aeddc

Fix cli x-seldon-route and also step tensor map and add step pipeline…

f13e89a

… example

sakoush reviewed Dec 14, 2022

View reviewed changes

sakoush approved these changes Dec 15, 2022

View reviewed changes

ukclivecox added the v2 label Dec 17, 2022

ukclivecox and others added 14 commits December 17, 2022 17:26

Update notebook

969e0b2

Ensure tensorMap works across pipelines by sending pipeline name in t…

83f91a5

…ensormap

Update docs/source/contents/pipelines/index.md

f84a73c

Co-authored-by: Sherif Akoush <sherif.akoush@gmail.com>

Update docs/source/contents/pipelines/index.md

a5fd2dd

Co-authored-by: Sherif Akoush <sherif.akoush@gmail.com>

Bump MLServer version to 1.2.1 (#4503)

bc85dbd

add a notebook test for changing model replicas (#4504)

39fdf04

Ensure x-request-id header matches kafka key (#4511)

5663bac

Fix possible SIGSEV after producer close in modelgateway (#4515)

e1930d2

* Fix possible SIGSEV after producer close in modelgateway * Set running after setup * review comments

Merge branch 'v2' into pipeline-inputs

7cd32e8

review comments

29f1aa4

sakoush approved these changes Dec 29, 2022

View reviewed changes

ukclivecox merged commit 5209861 into SeldonIO:v2 Jan 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pipeline inputs #4499

Pipeline inputs #4499

ukclivecox commented Dec 12, 2022 •

edited

review-notebook-app bot commented Dec 12, 2022

sakoush left a comment

sakoush Dec 14, 2022

sakoush left a comment

sakoush Dec 14, 2022

ukclivecox Dec 29, 2022

sakoush Dec 14, 2022

ukclivecox Dec 29, 2022

sakoush Dec 14, 2022

ukclivecox Dec 29, 2022

sakoush left a comment

sakoush Dec 15, 2022

seldondev commented Dec 15, 2022

sakoush left a comment


		Further examples can be found in the [pipeline-to-pipeline examples](../examples/pipeline-to-pipeline.md).

		Present caveats:

Pipeline inputs #4499

Pipeline inputs #4499

Conversation

ukclivecox commented Dec 12, 2022 • edited

review-notebook-app bot commented Dec 12, 2022

sakoush left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sakoush left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sakoush left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seldondev commented Dec 15, 2022

sakoush left a comment

Choose a reason for hiding this comment

ukclivecox commented Dec 12, 2022 •

edited