Skip to content

Add sink.uri#207

Merged
benjaminhuo merged 2 commits intoOpenFunction:mainfrom
rainbend:dev
Jan 10, 2022
Merged

Add sink.uri#207
benjaminhuo merged 2 commits intoOpenFunction:mainfrom
rainbend:dev

Conversation

@rainbend
Copy link
Copy Markdown
Member

The solutions are as follows:

type SinkSpec struct {
 	Ref *Reference `json:"ref,omitempty"`
 	Uri *string    `json:"uri,omitempty"`
}
 
type Reference struct {
 	Kind       string `json:"kind"`
 	Namespace  string `json:"namespace,omitempty"`
 	Name       string `json:"name"`
 	APIVersion string `json:"apiVersion"`
}

Reference provides references to Knative Service and Function resources from which to retrieve function URLs Uri is used to pass the URL directly

When using the Sink, you must use one of these attributes, and in priority Uri > Reference

Resolves #198

Signed-off-by: zhangwei [email protected]

Signed-off-by: zhangwei <[email protected]>
}

// SinkSpec describes an event source for the Kafka.
// SinkSpec describes an event source for the Kafka, priority Uri > Reference.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@arugal Thanks for the PR, the test part is professional.

The original comment is wrong actually, would you help to correct it?
// SinkSpec describes an event source for the Kafka, priority Uri > Reference.
=>
// SinkSpec specifies the receiver of the events an EventSource received, sinks in the Uri format have higher priority than sinks in Reference format.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done

@benjaminhuo
Copy link
Copy Markdown
Member

benjaminhuo commented Jan 10, 2022

This PR looks good to me now.
After we add an URL to the function status as described in #210, we can update the reference part to support using the OpenFunction reference to retrieve the function URL direct.

Signed-off-by: zhangwei <[email protected]>
Comment on lines +184 to +192
} else {
var ksvc kservingv1.Service
if err := c.Get(ctx, types.NamespacedName{Namespace: sink.Ref.Namespace, Name: sink.Ref.Name}, &ksvc); err != nil {
log.Error(err, "Failed to find Knative Service", "namespace", sink.Ref.Namespace, "name", sink.Ref.Name)
return nil, err
}
url = ksvc.Status.URL.String()
namespace = sink.Ref.Namespace
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we also need to add support for Function in this section, what do you think?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

apiVersion: core.openfunction.io/v1alpha2
kind: Function
name: function-sample
namespace: default

like this ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes

@benjaminhuo benjaminhuo merged commit 165b826 into OpenFunction:main Jan 10, 2022
@rainbend rainbend deleted the dev branch January 10, 2022 06:28
@benjaminhuo
Copy link
Copy Markdown
Member

This PR looks good to me now. After we add an URL to the function status as described in #210, we can update the reference part to support using the OpenFunction reference to retrieve the function URL direct.

@tpiperatgod we can add URL to function status and then update this part

@benjaminhuo
Copy link
Copy Markdown
Member

benjaminhuo commented Jan 13, 2022

We already have an URL field in the function status in v0.5.0, so the reference can be an OpenFunction's own Function CRD now. So it's ok to add Function reference now @arugal , my appology
https://github.com/OpenFunction/OpenFunction/blob/main/apis/core/v1alpha2/function_types.go#L202

@rainbend
Copy link
Copy Markdown
Member Author

We already have an URL field in the function status in v0.5.0, so the reference can be an OpenFunction's own Function CRD now. So it's ok to add Function reference now @arugal , my appology https://github.com/OpenFunction/OpenFunction/blob/main/apis/core/v1alpha2/function_types.go#L202

I will submit a new PR

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.

Replace the sink.ref in events api with function ingress

3 participants