Set DeadLetterTopic when it and subScriptionName are empty #595
Conversation
| @@ -88,6 +88,12 @@ func (r *Function) Default() { | |||
| r.Spec.Namespace = DefaultNamespace | |||
| } | |||
|
|
|||
| if r.Spec.SubscriptionName == "" && r.Spec.DeadLetterTopic == "" { | |||
There was a problem hiding this comment.
We will need to set the subs name only if maxMessageRetry is set, which means the RetryDetails will provide to the function runtime.
There was a problem hiding this comment.
if function is created without maxMessageRetry or DeadLetterTopic, then sub is set to tenant-namespace-name, but when function is updated with maxMessageRetry and DeadLetterTopic provided, then the sub will be changed to tenant/namespace/name, which will make function consume from input topics again?
look like we should set the SubscriptionName(like tenant-namespace-name) once it is not defined
There was a problem hiding this comment.
We may need to consider the backward compatibility as well, if we change the default sub name and released, when users upgrade function-mesh, their existing function pods may change the sub name and cause duplicate consumption
looks like the better way is to set the deadLetterTopic name
There was a problem hiding this comment.
agree, we should set the deadLetterTopic when
maxMessageRetryis providedDeadLetterTopicis emptySubscriptionNameis empty or contains/
jiangpengcheng
force-pushed
the
set_default_ss_name
branch
from
ed3f3ed
to
8fe2dba
Compare
jiangpengcheng
changed the title
jiangpengcheng
force-pushed
the
set_default_ss_name
branch
from
8fe2dba
to
5e6e1ba
Compare
make sense, maybe it's better to do it without webhook enabled |
(If this PR fixes a github issue, please add
Fixes #<xyz>.)Fixes #593
(or if this PR is one task of a github issue, please add
Master Issue: #<xyz>to link to the master issue.)Master Issue: #
Motivation
Explain here the context, and why you're making that change. What is the problem you're trying to solve.
Modifications
Describe the modifications you've done.
Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Documentation
Check the box below.
Need to update docs?
doc-required(If you need help on updating docs, create a doc issue)
no-need-doc(Please explain why)
doc(If this PR contains doc changes)