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

add more params to dapr pubsub subscription #332

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

halegreen
Copy link

@halegreen halegreen commented Jul 23, 2022

fix issue: #329

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@halegreen
Copy link
Author

@tpiperatgod @webup @wrongerror hi, any suggestions on this pr?

PubSubRoutingRule *PubSubRoutingRule `json:"pubSubRoutingRule,omitempty"`
}

type PubSubRoutingRule struct {
Copy link
Member

Choose a reason for hiding this comment

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

@ZoeShaw101 Thanks for your contribution and sorry for our late response because we're busy on releasing v0.7.0

I think the PubSubRoutingRule can be changed to Subscription directly and we needn't define our own Subscription struct, simply referencing Dapr's Subsription is ok. https://github.com/dapr/go-sdk/blob/main/service/common/type.go#L89

// Subscription represents single topic subscription.
type Subscription struct {
	// PubsubName is name of the pub/sub this message came from
	PubsubName string `json:"pubsubname"`
	// Topic is the name of the topic
	Topic string `json:"topic"`
	// Metadata is the subscription metadata
	Metadata map[string]string `json:"metadata,omitempty"`
	// Route is the route of the handler where HTTP topic events should be published (passed as Path in gRPC)
	Route string `json:"route"`
	// Match is the CEL expression to match on the CloudEvent envelope.
	Match string `json:"match"`
	// Priority is the priority in which to evaluate the match (lower to higher).
	Priority int `json:"priority"`
	// DisableTopicValidation allows to receive events from publisher topics that differ from the subscribed topic.
	DisableTopicValidation bool `json:"disableTopicValidation"`
}

The PubsubName, and Topic fields of Subscription will not be used because OpenFunction defined them in DaprIO https://github.com/OpenFunction/OpenFunction/blob/main/apis/core/v1beta1/serving_types.go#L91

And Route will not be used either because the user needn't set this by himself, it's handled automatically by the functions-framework implementation.

What are valuable to OpenFunction are Subscription's metadata, Match, Priority, and DisableTopicValidation fields which need to be passed to functions-framework implementations via context.

@wanjunlei @tpiperatgod @wrongerror @webup @lizzzcai , what do you think?

Copy link
Author

Choose a reason for hiding this comment

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

fixed it , please have a look @benjaminhuo , tks

@benjaminhuo
Copy link
Member

@ZoeShaw101 Also don't forget to sign your commit by

git commit -s --amend
git push -f

Signed-off-by: WangXiaoxiao <1141195807@qq.com>
@halegreen halegreen force-pushed the add-more-params-to-dapr-pubsub-subscription branch from baa54b9 to 5d51253 Compare February 19, 2023 07:40
@halegreen halegreen requested review from benjaminhuo and removed request for tpiperatgod, wrongerror and wanjunlei February 19, 2023 07:41
@halegreen
Copy link
Author

Hi, any review updates here? @benjaminhuo @zhbinary @tpiperatgod

@Mershab99
Copy link

Is it possible to add Bulk Subscriptions to this PR? #519

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