You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by GitBox <gi...@apache.org> on 2020/02/29 15:11:07 UTC
[GitHub] [servicecomb-kie] zhulijian1 opened a new pull request #107: topic
support none label cases
zhulijian1 opened a new pull request #107: topic support none label cases
URL: https://github.com/apache/servicecomb-kie/pull/107
Fixes #103
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-kie] zhulijian1 commented on a change in pull request
#107: #103 topic support none label cases
Posted by GitBox <gi...@apache.org>.
zhulijian1 commented on a change in pull request #107: #103 topic support none label cases
URL: https://github.com/apache/servicecomb-kie/pull/107#discussion_r386430741
##########
File path: server/pubsub/struct.go
##########
@@ -81,6 +85,9 @@ func ParseTopicString(s string) (*Topic, error) {
}
//Match compare event with topic
+//If the match type is set to exact in long pulling request,
+//only update request with exactly the same label will match the request and will trigger an immediate return.
+//If no labels are set in query params, only the none label update request will match.
Review comment:
fixed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-kie] zhulijian1 closed pull request #107: #103 topic
support none label cases
Posted by GitBox <gi...@apache.org>.
zhulijian1 closed pull request #107: #103 topic support none label cases
URL: https://github.com/apache/servicecomb-kie/pull/107
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-kie] zhulijian1 commented on a change in pull request
#107: #103 topic support none label cases
Posted by GitBox <gi...@apache.org>.
zhulijian1 commented on a change in pull request #107: #103 topic support none label cases
URL: https://github.com/apache/servicecomb-kie/pull/107#discussion_r386340442
##########
File path: server/pubsub/struct.go
##########
@@ -86,11 +90,15 @@ func (t *Topic) Match(event *KVChangeEvent) bool {
return false
}
}
- for k, v := range t.Labels {
- if event.Labels[k] != v {
- return false
- }
+ if len(t.Labels) == 0 {
match = true
Review comment:
已修改
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-kie] zhulijian1 opened a new pull request #107: #103
topic support none label cases
Posted by GitBox <gi...@apache.org>.
zhulijian1 opened a new pull request #107: #103 topic support none label cases
URL: https://github.com/apache/servicecomb-kie/pull/107
Fixes #103
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull
request #107: #103 topic support none label cases
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #107: #103 topic support none label cases
URL: https://github.com/apache/servicecomb-kie/pull/107#discussion_r386231616
##########
File path: server/pubsub/struct.go
##########
@@ -60,14 +62,16 @@ func ParseTopicString(s string) (*Topic, error) {
if err != nil {
return nil, err
}
- ls := strings.Split(t.LabelsFormat, "::")
- if len(ls) != 0 {
- for _, l := range ls {
- s := strings.Split(l, "=")
- if len(s) != 2 {
- return nil, errors.New("invalid label:" + l)
+ if t.LabelsFormat != stringutil.LabelNone {
Review comment:
if t.LabelsFormat != stringutil.LabelNone 是一种稀有出现条件，不如编写if t.LabelsFormat -= stringutil.LabelNone 然后返回topic,nil 代码更加简洁，减少了嵌套
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-kie] tianxiaoliang merged pull request #107: #103
topic support none label cases
Posted by GitBox <gi...@apache.org>.
tianxiaoliang merged pull request #107: #103 topic support none label cases
URL: https://github.com/apache/servicecomb-kie/pull/107
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-kie] zhulijian1 opened a new pull request #107: #103
topic support none label cases
Posted by GitBox <gi...@apache.org>.
zhulijian1 opened a new pull request #107: #103 topic support none label cases
URL: https://github.com/apache/servicecomb-kie/pull/107
Fixes #103
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-kie] zhulijian1 closed pull request #107: #103 topic
support none label cases
Posted by GitBox <gi...@apache.org>.
zhulijian1 closed pull request #107: #103 topic support none label cases
URL: https://github.com/apache/servicecomb-kie/pull/107
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull
request #107: #103 topic support none label cases
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #107: #103 topic support none label cases
URL: https://github.com/apache/servicecomb-kie/pull/107#discussion_r386385117
##########
File path: server/pubsub/struct.go
##########
@@ -81,6 +85,9 @@ func ParseTopicString(s string) (*Topic, error) {
}
//Match compare event with topic
+//If the match type is set to exact in long pulling request,
+//only update request with exactly the same label will match the request and will trigger an immediate return.
+//If no labels are set in query params, only the none label update request will match.
Review comment:
第三行注释，不完全对，并不是only 而是 none label和label子集都会match
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-kie] zhulijian1 commented on a change in pull request
#107: #103 topic support none label cases
Posted by GitBox <gi...@apache.org>.
zhulijian1 commented on a change in pull request #107: #103 topic support none label cases
URL: https://github.com/apache/servicecomb-kie/pull/107#discussion_r386340493
##########
File path: server/pubsub/struct.go
##########
@@ -60,14 +62,16 @@ func ParseTopicString(s string) (*Topic, error) {
if err != nil {
return nil, err
}
- ls := strings.Split(t.LabelsFormat, "::")
- if len(ls) != 0 {
- for _, l := range ls {
- s := strings.Split(l, "=")
- if len(s) != 2 {
- return nil, errors.New("invalid label:" + l)
+ if t.LabelsFormat != stringutil.LabelNone {
Review comment:
已修改
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-kie] tianxiaoliang commented on issue #107: #103 topic
support none label cases
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on issue #107: #103 topic support none label cases
URL: https://github.com/apache/servicecomb-kie/pull/107#issuecomment-593396351
没别的问题了 ，增量commit下注释就可以合入了
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull
request #107: #103 topic support none label cases
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #107: #103 topic support none label cases
URL: https://github.com/apache/servicecomb-kie/pull/107#discussion_r386230806
##########
File path: server/pubsub/struct.go
##########
@@ -86,11 +90,15 @@ func (t *Topic) Match(event *KVChangeEvent) bool {
return false
}
}
- for k, v := range t.Labels {
- if event.Labels[k] != v {
- return false
- }
+ if len(t.Labels) == 0 {
match = true
Review comment:
另外这里的逻辑已经开始变得复杂了，请对Match这个方法级注释中添加行为说明
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull
request #107: #103 topic support none label cases
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #107: #103 topic support none label cases
URL: https://github.com/apache/servicecomb-kie/pull/107#discussion_r386230487
##########
File path: server/pubsub/struct.go
##########
@@ -86,11 +90,15 @@ func (t *Topic) Match(event *KVChangeEvent) bool {
return false
}
}
- for k, v := range t.Labels {
- if event.Labels[k] != v {
- return false
- }
+ if len(t.Labels) == 0 {
match = true
Review comment:
这里有个问题，MatchType 不生效，如果watch的label为空的话，当要求exactmatch的手，过来的时间也必须得是label为空，才能触发
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull
request #107: #103 topic support none label cases
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #107: #103 topic support none label cases
URL: https://github.com/apache/servicecomb-kie/pull/107#discussion_r386230487
##########
File path: server/pubsub/struct.go
##########
@@ -86,11 +90,15 @@ func (t *Topic) Match(event *KVChangeEvent) bool {
return false
}
}
- for k, v := range t.Labels {
- if event.Labels[k] != v {
- return false
- }
+ if len(t.Labels) == 0 {
match = true
Review comment:
这里有个问题，MatchType 不生效，如果watch的label为空的话，当要求exactmatch的手，过来的事件也必须得是label为空，才能触发
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services