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 2021/11/22 03:50:39 UTC
[GitHub] [servicecomb-kie] robotLJW opened a new pull request #228: Feature: Adding, deleting and modifying operations to add tasks when …
robotLJW opened a new pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228
…synchronization is turned on
A task will be added when the configuration is added or modified successfully
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757982436
##########
File path: server/service/kv/kv_svc.go
##########
@@ -67,7 +68,7 @@ func ListKV(ctx context.Context, request *model.ListKVRequest) (int64, *model.KV
//Create get latest revision from history
//and increase revision of label
//and insert key
-func Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, *errsvc.Error) {
+func Create(ctx context.Context, kv *model.KVDoc, syncEnabled bool) (*model.KVDoc, *errsvc.Error) {
Review comment:
这块要透传函数式参数，不能写死bool，以后没有扩展性
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] robotLJW commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r756791426
##########
File path: server/datasource/etcd/kv/kv_dao.go
##########
@@ -59,6 +61,100 @@ func (s *Dao) Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
return kv, nil
}
+// CreateWithTask is used to create a task when the configuration is created
+func (s *Dao) CreateWithTask(ctx context.Context, kv *model.KVDoc, task *model.Task) (*model.KVDoc, error) {
+ kvBytes, err := json.Marshal(kv)
+ if err != nil {
+ openlog.Error("fail to marshal kv")
+ return nil, err
+ }
+ taskBytes, err := json.Marshal(task)
+ if err != nil {
+ openlog.Error("fail to marshal task ")
+ return nil, err
+ }
+ kvOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.KV(kv.Domain, kv.Project, kv.ID)), etcdadpt.WithValue(kvBytes))
+ taskOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.TaskKey(kv.Domain, kv.Project, strconv.FormatInt(task.Timestamp, 10), task.TaskID)), etcdadpt.WithValue(taskBytes))
+ resp, err := etcdadpt.Instance().TxnWithCmp(ctx, []etcdadpt.OpOptions{kvOpPut, taskOpPut}, nil, nil)
Review comment:
已经改成txn
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] robotLJW commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757182016
##########
File path: server/datasource/dao.go
##########
@@ -70,13 +77,14 @@ func GetBroker() Broker {
//KVDao provide api of KV entity
type KVDao interface {
// Create Update List are usually for admin console
- Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
- Update(ctx context.Context, kv *model.KVDoc) error
+ Create(ctx context.Context, kv *model.KVDoc, options ...CUDOption) (*model.KVDoc, error)
Review comment:
这边考虑R 查找不需要写任务就能加上去了
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] robotLJW commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757349083
##########
File path: server/datasource/etcd/kv/kv_dao_trans.go
##########
@@ -0,0 +1,254 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package kv
+
+import (
+ "context"
+ "encoding/json"
+
+ "github.com/go-chassis/cari/sync"
+ "github.com/go-chassis/openlog"
+ "github.com/little-cui/etcdadpt"
+
+ "github.com/apache/servicecomb-kie/pkg/model"
+ "github.com/apache/servicecomb-kie/server/datasource"
+ "github.com/apache/servicecomb-kie/server/datasource/etcd/key"
+)
+
+// CreateWithTask is to start transaction when creating KV, will create task in a transaction operation
+func (s *Dao) CreateWithTask(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error) {
+ kvBytes, err := json.Marshal(kv)
+ if err != nil {
+ openlog.Error("marshal kv ")
+ return nil, err
+ }
+ task, err := datasource.NewTask(sync.CreateAction, datasource.ConfigResource)
+ if err != nil {
+ openlog.Error("fail to create task" + err.Error())
+ return nil, err
+ }
+ task.Data = kv
+ taskBytes, err := json.Marshal(task)
+ if err != nil {
+ openlog.Error("fail to marshal task ")
+ return nil, err
+ }
+ kvOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.KV(kv.Domain, kv.Project, kv.ID)), etcdadpt.WithValue(kvBytes))
+ taskOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.TaskKey(kv.Domain, kv.Project, task.TaskID, task.Timestamp)), etcdadpt.WithValue(taskBytes))
+ err = etcdadpt.Txn(ctx, []etcdadpt.OpOptions{kvOpPut, taskOpPut})
Review comment:
已经改成私有函数，将kv_dao_trans.go删除
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757182484
##########
File path: server/datasource/dao.go
##########
@@ -38,14 +41,18 @@ var (
ErrKeyNotExists = errors.New("can not find any key value")
ErrRecordNotExists = errors.New("can not find any polling data")
ErrRevisionNotExist = errors.New("revision does not exist")
- ErrAliasNotGiven = errors.New("label alias not given")
ErrKVAlreadyExists = errors.New("kv already exists")
ErrTooMany = errors.New("key with labels should be only one")
)
const (
DefaultValueType = "text"
MaxHistoryNum = 100
+
+ CreateAction = "create"
Review comment:
cari里已经有了，不需要重复定义
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757260334
##########
File path: server/service/kv/kv_svc_test.go
##########
@@ -178,3 +180,93 @@ func TestService_Delete(t *testing.T) {
assert.NoError(t, err)
})
}
+
+// Test the situation after the synchronization is turned on
+func TestSync(t *testing.T) {
+
+ t.Run("do some initialization to start the sync test", func(t *testing.T) {
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] little-cui commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
little-cui commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r756507891
##########
File path: pkg/model/db_schema.go
##########
@@ -48,6 +48,25 @@ type KVDoc struct {
Domain string `json:"domain,omitempty" yaml:"domain,omitempty" validate:"min=1,max=256,commonName"` //redundant
}
+// Task is db struct to store sync task
+type Task struct {
+ TaskID string `json:"task_id" bson:"task_id"`
+ Action string `json:"action" bson:"action"`
+ DataType string `json:"data_type" bson:"data_type"`
+ Data interface{} `json:"data" bson:"data"`
+ Timestamp int64 `json:"timestamp" bson:"timestamp"`
+ Status string `json:"status" bson:"status"`
+}
+
+// Tombstone is db struct to store the deleted resource information
+type Tombstone struct {
Review comment:
Task和Tombstone都是公共类型，可以放到CARI仓库
##########
File path: .github/workflows/mongo_storage.yml
##########
@@ -13,11 +13,8 @@ jobs:
uses: actions/checkout@v1
- name: UT
run: |
- cd build
- time bash build_docker.sh
- cd ../
- sudo docker-compose -f ./deployments/docker/docker-compose.yaml up -d
- sleep 20
+ sudo docker-compose -f ./deployments/docker/mongo-compose.yaml up -d
+ sleep 15
export TEST_DB_KIND=mongo
- export TEST_DB_URI=mongodb://kie:123@127.0.0.1:27017/kie
+ export TEST_DB_URI=mongodb://kie:123@127.0.0.1:30001/kie
Review comment:
修改端口的原因是什么？
##########
File path: server/datasource/etcd/kv/kv_dao.go
##########
@@ -59,6 +61,100 @@ func (s *Dao) Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
return kv, nil
}
+// CreateWithTask is used to create a task when the configuration is created
+func (s *Dao) CreateWithTask(ctx context.Context, kv *model.KVDoc, task *model.Task) (*model.KVDoc, error) {
+ kvBytes, err := json.Marshal(kv)
+ if err != nil {
+ openlog.Error("fail to marshal kv")
+ return nil, err
+ }
+ taskBytes, err := json.Marshal(task)
+ if err != nil {
+ openlog.Error("fail to marshal task ")
+ return nil, err
+ }
+ kvOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.KV(kv.Domain, kv.Project, kv.ID)), etcdadpt.WithValue(kvBytes))
+ taskOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.TaskKey(kv.Domain, kv.Project, strconv.FormatInt(task.Timestamp, 10), task.TaskID)), etcdadpt.WithValue(taskBytes))
+ resp, err := etcdadpt.Instance().TxnWithCmp(ctx, []etcdadpt.OpOptions{kvOpPut, taskOpPut}, nil, nil)
+ if err != nil {
+ openlog.Error("create error", openlog.WithTags(openlog.Tags{
+ "err": err.Error(),
+ "kv": kv,
+ "task": task,
+ }))
+ return nil, err
+ }
+ if !resp.Succeeded {
Review comment:
没有cmp语义，这个条件不存在，其他地方一并修改
##########
File path: server/datasource/etcd/kv/kv_dao.go
##########
@@ -59,6 +61,100 @@ func (s *Dao) Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
return kv, nil
}
+// CreateWithTask is used to create a task when the configuration is created
+func (s *Dao) CreateWithTask(ctx context.Context, kv *model.KVDoc, task *model.Task) (*model.KVDoc, error) {
+ kvBytes, err := json.Marshal(kv)
+ if err != nil {
+ openlog.Error("fail to marshal kv")
+ return nil, err
+ }
+ taskBytes, err := json.Marshal(task)
+ if err != nil {
+ openlog.Error("fail to marshal task ")
+ return nil, err
+ }
+ kvOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.KV(kv.Domain, kv.Project, kv.ID)), etcdadpt.WithValue(kvBytes))
+ taskOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.TaskKey(kv.Domain, kv.Project, strconv.FormatInt(task.Timestamp, 10), task.TaskID)), etcdadpt.WithValue(taskBytes))
+ resp, err := etcdadpt.Instance().TxnWithCmp(ctx, []etcdadpt.OpOptions{kvOpPut, taskOpPut}, nil, nil)
Review comment:
使用etcdadpt.Txn()代替，其他地方一并修改
##########
File path: server/datasource/etcd/key/key.go
##########
@@ -28,8 +28,27 @@ const (
keyCounter = "counter"
keyHistory = "kv-history"
keyTrack = "track"
+ syncer = "syncer"
+ task = "task"
+ tombstone = "tombstone"
)
+func getSyncRootKey() string {
+ return split + syncer + split + task
+}
+
+func getTombstoneRootKey() string {
+ return split + tombstone
+}
+
+func TaskKey(domain, project, timestamp, taskID string) string {
Review comment:
timestamp定义为int64，使用更方便
##########
File path: server/datasource/dao.go
##########
@@ -73,10 +74,20 @@ type KVDao interface {
Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
Update(ctx context.Context, kv *model.KVDoc) error
List(ctx context.Context, project, domain string, options ...FindOption) (*model.KVResponse, error)
+ // CreateWithTask will create the task when the configuration is created
+ CreateWithTask(ctx context.Context, kv *model.KVDoc, task *model.Task) (*model.KVDoc, error)
+ // UpdateWithTask will create the task when the configuration is updated
+ UpdateWithTask(ctx context.Context, kv *model.KVDoc, task *model.Task) error
+
//FindOneAndDelete deletes one kv by id and return the deleted kv as these appeared before deletion
FindOneAndDelete(ctx context.Context, kvID string, project, domain string) (*model.KVDoc, error)
//FindManyAndDelete deletes multiple kvs and return the deleted kv list as these appeared before deletion
FindManyAndDelete(ctx context.Context, kvIDs []string, project, domain string) ([]*model.KVDoc, int64, error)
+ // FindOneAndDeleteWithTask will create the task when the configuration is deleted
+ FindOneAndDeleteWithTask(ctx context.Context, kvID string, project, domain string, task *model.Task, tombstone *model.Tombstone) (*model.KVDoc, error)
+ // FindManyAndDeleteWithTask will create tasks when many configurations are deleted
+ FindManyAndDeleteWithTask(ctx context.Context, kvIDs []string, project, domain string, tasks []*model.Task, tombstones []*model.Tombstone) ([]*model.KVDoc, int64, error)
Review comment:
task, tombstone不需要外部传递，只要是事务写逻辑，都应该在DAO层内生成task, tombstone
##########
File path: server/datasource/dao.go
##########
@@ -73,10 +74,20 @@ type KVDao interface {
Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
Update(ctx context.Context, kv *model.KVDoc) error
List(ctx context.Context, project, domain string, options ...FindOption) (*model.KVResponse, error)
+ // CreateWithTask will create the task when the configuration is created
+ CreateWithTask(ctx context.Context, kv *model.KVDoc, task *model.Task) (*model.KVDoc, error)
+ // UpdateWithTask will create the task when the configuration is updated
+ UpdateWithTask(ctx context.Context, kv *model.KVDoc, task *model.Task) error
Review comment:
使用 options pattern扩展原有接口，如：WithTransation(True)，不建议增加新接口
##########
File path: server/service/kv/kv_svc.go
##########
@@ -250,9 +312,26 @@ func Update(ctx context.Context, kv *model.UpdateKVRequest) (*model.KVDoc, error
}
func FindOneAndDelete(ctx context.Context, kvID string, project, domain string) (*model.KVDoc, error) {
- kv, err := datasource.GetBroker().GetKVDao().FindOneAndDelete(ctx, kvID, project, domain)
- if err != nil {
- return nil, err
+ var kv *model.KVDoc
+ var err error
+ if cfg.GetSync().Enabled {
Review comment:
用了options pattern之后就消除了if
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] robotLJW commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r756752683
##########
File path: pkg/model/db_schema.go
##########
@@ -48,6 +48,25 @@ type KVDoc struct {
Domain string `json:"domain,omitempty" yaml:"domain,omitempty" validate:"min=1,max=256,commonName"` //redundant
}
+// Task is db struct to store sync task
+type Task struct {
+ TaskID string `json:"task_id" bson:"task_id"`
+ Action string `json:"action" bson:"action"`
+ DataType string `json:"data_type" bson:"data_type"`
+ Data interface{} `json:"data" bson:"data"`
+ Timestamp int64 `json:"timestamp" bson:"timestamp"`
+ Status string `json:"status" bson:"status"`
+}
+
+// Tombstone is db struct to store the deleted resource information
+type Tombstone struct {
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] robotLJW commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757178149
##########
File path: .github/workflows/mongo_storage.yml
##########
@@ -13,10 +13,7 @@ jobs:
uses: actions/checkout@v1
- name: UT
run: |
- cd build
- time bash build_docker.sh
Review comment:
跑用例ci的时候不需要制作镜像的，不用到sc自身镜像，只要用mongo就可以了
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] robotLJW commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757256665
##########
File path: server/datasource/etcd/kv/kv_dao_trans.go
##########
@@ -0,0 +1,239 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package kv
+
+import (
+ "context"
+ "encoding/json"
+
+ "github.com/go-chassis/cari/sync"
+ "github.com/go-chassis/openlog"
+ "github.com/little-cui/etcdadpt"
+
+ "github.com/apache/servicecomb-kie/pkg/model"
+ "github.com/apache/servicecomb-kie/server/datasource"
+ "github.com/apache/servicecomb-kie/server/datasource/etcd/key"
+)
+
+func (s *Dao) createWithTask(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error) {
+ kvBytes, err := json.Marshal(kv)
+ if err != nil {
+ openlog.Error("marshal kv ")
+ return nil, err
+ }
+ task, err := datasource.NewTask(datasource.CreateAction, datasource.ConfigResource)
+ if err != nil {
+ openlog.Error("fail to create task" + err.Error())
+ return nil, err
+ }
+ task.Data = kv
+ taskBytes, err := json.Marshal(task)
+ if err != nil {
+ openlog.Error("fail to marshal task ")
+ return nil, err
+ }
+ kvOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.KV(kv.Domain, kv.Project, kv.ID)), etcdadpt.WithValue(kvBytes))
+ taskOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.TaskKey(kv.Domain, kv.Project, task.TaskID, task.Timestamp)), etcdadpt.WithValue(taskBytes))
+ err = etcdadpt.Txn(ctx, []etcdadpt.OpOptions{kvOpPut, taskOpPut})
+ if err != nil {
+ openlog.Error("create error", openlog.WithTags(openlog.Tags{
+ "err": err.Error(),
+ "kv": kv,
+ "task": task,
+ }))
+ return nil, err
+ }
+ return kv, nil
+}
+
+func (s *Dao) updateWithTask(ctx context.Context, kv *model.KVDoc) error {
+ keyKV := key.KV(kv.Domain, kv.Project, kv.ID)
+ resp, err := etcdadpt.Get(ctx, keyKV)
+ if err != nil {
+ openlog.Error(err.Error())
+ return err
+ }
+ if resp == nil {
+ return datasource.ErrKeyNotExists
+ }
+
+ var old model.KVDoc
+ err = json.Unmarshal(resp.Value, &old)
+ if err != nil {
+ openlog.Error(err.Error())
+ return err
+ }
+ old.LabelFormat = kv.LabelFormat
+ old.Value = kv.Value
+ old.Status = kv.Status
+ old.Checker = kv.Checker
+ old.UpdateTime = kv.UpdateTime
+ old.UpdateRevision = kv.UpdateRevision
+
+ bytes, err := json.Marshal(old)
+ if err != nil {
+ openlog.Error(err.Error())
+ return err
+ }
+ task, err := datasource.NewTask(datasource.UpdateAction, datasource.ConfigResource)
+ if err != nil {
+ openlog.Error("fail to create task" + err.Error())
+ return err
+ }
+ // update data
+ task.Data = old
+ taskBytes, err := json.Marshal(task)
+ if err != nil {
+ openlog.Error(err.Error())
+ return err
+ }
+ kvOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(keyKV), etcdadpt.WithValue(bytes))
+ taskOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.TaskKey(kv.Domain, kv.Project, task.TaskID, task.Timestamp)), etcdadpt.WithValue(taskBytes))
+ err = etcdadpt.Txn(ctx, []etcdadpt.OpOptions{kvOpPut, taskOpPut})
+ if err != nil {
+ openlog.Error("update error", openlog.WithTags(openlog.Tags{
+ "err": err.Error(),
+ "kv": kv,
+ "task": task,
+ }))
+ return err
+ }
+ return nil
+}
+
+func (s *Dao) findOneAndDeleteWithTask(ctx context.Context, kvID, project, domain string) (*model.KVDoc, error) {
+ kvKey := key.KV(domain, project, kvID)
+ kvDoc := model.KVDoc{}
+ kvResp, err := etcdadpt.Get(ctx, kvKey)
+ if err != nil {
+ openlog.Error(err.Error())
+ return nil, err
+ }
+ if kvResp == nil {
+ return nil, datasource.ErrKeyNotExists
+ }
+ err = json.Unmarshal(kvResp.Value, &kvDoc)
+ if err != nil {
+ openlog.Error("fail to unmarshal kv" + err.Error())
+ return nil, err
+ }
+ // create task
+ task, err := datasource.NewTask(datasource.DeleteAction, datasource.ConfigResource)
+ if err != nil {
+ openlog.Error("fail to create task" + err.Error())
+ return nil, err
+ }
+
+ task.Data = kvDoc
+ taskBytes, err := json.Marshal(task)
+ if err != nil {
+ openlog.Error("fail to marshal task" + err.Error())
+ return nil, err
+ }
+ // create tombstone
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757185948
##########
File path: server/datasource/etcd/kv/kv_dao_trans.go
##########
@@ -0,0 +1,239 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package kv
+
+import (
+ "context"
+ "encoding/json"
+
+ "github.com/go-chassis/cari/sync"
+ "github.com/go-chassis/openlog"
+ "github.com/little-cui/etcdadpt"
+
+ "github.com/apache/servicecomb-kie/pkg/model"
+ "github.com/apache/servicecomb-kie/server/datasource"
+ "github.com/apache/servicecomb-kie/server/datasource/etcd/key"
+)
+
+func (s *Dao) createWithTask(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error) {
+ kvBytes, err := json.Marshal(kv)
+ if err != nil {
+ openlog.Error("marshal kv ")
+ return nil, err
+ }
+ task, err := datasource.NewTask(datasource.CreateAction, datasource.ConfigResource)
+ if err != nil {
+ openlog.Error("fail to create task" + err.Error())
+ return nil, err
+ }
+ task.Data = kv
+ taskBytes, err := json.Marshal(task)
+ if err != nil {
+ openlog.Error("fail to marshal task ")
+ return nil, err
+ }
+ kvOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.KV(kv.Domain, kv.Project, kv.ID)), etcdadpt.WithValue(kvBytes))
+ taskOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.TaskKey(kv.Domain, kv.Project, task.TaskID, task.Timestamp)), etcdadpt.WithValue(taskBytes))
+ err = etcdadpt.Txn(ctx, []etcdadpt.OpOptions{kvOpPut, taskOpPut})
+ if err != nil {
+ openlog.Error("create error", openlog.WithTags(openlog.Tags{
+ "err": err.Error(),
+ "kv": kv,
+ "task": task,
+ }))
+ return nil, err
+ }
+ return kv, nil
+}
+
+func (s *Dao) updateWithTask(ctx context.Context, kv *model.KVDoc) error {
Review comment:
update应该发生在任务处理的过程中，而且只能更新状态字段。
北向接口接受的所有写操作，都是不断地在创建新task，而不是针对通过一个业务实体，根据创建和更新操作对同一个task做更新
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] little-cui commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
little-cui commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757723947
##########
File path: .github/workflows/mongo_storage.yml
##########
@@ -13,10 +13,7 @@ jobs:
uses: actions/checkout@v1
- name: UT
run: |
- cd build
Review comment:
建议另起一个并行的task，用于验证docker build的脚本，而不是因为没依赖而删掉
##########
File path: deployments/docker/mongo-compose.yaml
##########
@@ -0,0 +1,32 @@
+## ---------------------------------------------------------------------------
+## Licensed to the Apache Software Foundation (ASF) under one or more
+## contributor license agreements. See the NOTICE file distributed with
+## this work for additional information regarding copyright ownership.
+## The ASF licenses this file to You under the Apache License, Version 2.0
+## (the "License"); you may not use this file except in compliance with
+## the License. You may obtain a copy of the License at
+##
+## http://www.apache.org/licenses/LICENSE-2.0
+##
+## Unless required by applicable law or agreed to in writing, software
+## distributed under the License is distributed on an "AS IS" BASIS,
+## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+## See the License for the specific language governing permissions and
+## limitations under the License.
+## ---------------------------------------------------------------------------
+version: '3.3'
+services:
+ mongo:
+ image: mongo:4.2
+ command: mongod --replSet my-replica-set --port 27017 --bind_ip_all
Review comment:
此脚本直接替换example/dev/docker-compose.yaml会更好，而不是新增一个
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] robotLJW commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r756543007
##########
File path: server/datasource/etcd/key/key.go
##########
@@ -28,8 +28,27 @@ const (
keyCounter = "counter"
keyHistory = "kv-history"
keyTrack = "track"
+ syncer = "syncer"
+ task = "task"
+ tombstone = "tombstone"
)
+func getSyncRootKey() string {
+ return split + syncer + split + task
+}
+
+func getTombstoneRootKey() string {
+ return split + tombstone
+}
+
+func TaskKey(domain, project, timestamp, taskID string) string {
Review comment:
结构体里面其实是int64 这边主要是组装key，所以使用string
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757192064
##########
File path: server/datasource/etcd/kv/kv_dao_trans.go
##########
@@ -0,0 +1,239 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package kv
+
+import (
+ "context"
+ "encoding/json"
+
+ "github.com/go-chassis/cari/sync"
+ "github.com/go-chassis/openlog"
+ "github.com/little-cui/etcdadpt"
+
+ "github.com/apache/servicecomb-kie/pkg/model"
+ "github.com/apache/servicecomb-kie/server/datasource"
+ "github.com/apache/servicecomb-kie/server/datasource/etcd/key"
+)
+
+func (s *Dao) createWithTask(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error) {
+ kvBytes, err := json.Marshal(kv)
+ if err != nil {
+ openlog.Error("marshal kv ")
+ return nil, err
+ }
+ task, err := datasource.NewTask(datasource.CreateAction, datasource.ConfigResource)
+ if err != nil {
+ openlog.Error("fail to create task" + err.Error())
+ return nil, err
+ }
+ task.Data = kv
+ taskBytes, err := json.Marshal(task)
+ if err != nil {
+ openlog.Error("fail to marshal task ")
+ return nil, err
+ }
+ kvOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.KV(kv.Domain, kv.Project, kv.ID)), etcdadpt.WithValue(kvBytes))
+ taskOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.TaskKey(kv.Domain, kv.Project, task.TaskID, task.Timestamp)), etcdadpt.WithValue(taskBytes))
+ err = etcdadpt.Txn(ctx, []etcdadpt.OpOptions{kvOpPut, taskOpPut})
+ if err != nil {
+ openlog.Error("create error", openlog.WithTags(openlog.Tags{
+ "err": err.Error(),
+ "kv": kv,
+ "task": task,
+ }))
+ return nil, err
+ }
+ return kv, nil
+}
+
+func (s *Dao) updateWithTask(ctx context.Context, kv *model.KVDoc) error {
+ keyKV := key.KV(kv.Domain, kv.Project, kv.ID)
+ resp, err := etcdadpt.Get(ctx, keyKV)
+ if err != nil {
+ openlog.Error(err.Error())
+ return err
+ }
+ if resp == nil {
+ return datasource.ErrKeyNotExists
+ }
+
+ var old model.KVDoc
+ err = json.Unmarshal(resp.Value, &old)
+ if err != nil {
+ openlog.Error(err.Error())
+ return err
+ }
+ old.LabelFormat = kv.LabelFormat
+ old.Value = kv.Value
+ old.Status = kv.Status
+ old.Checker = kv.Checker
+ old.UpdateTime = kv.UpdateTime
+ old.UpdateRevision = kv.UpdateRevision
+
+ bytes, err := json.Marshal(old)
+ if err != nil {
+ openlog.Error(err.Error())
+ return err
+ }
+ task, err := datasource.NewTask(datasource.UpdateAction, datasource.ConfigResource)
+ if err != nil {
+ openlog.Error("fail to create task" + err.Error())
+ return err
+ }
+ // update data
+ task.Data = old
+ taskBytes, err := json.Marshal(task)
+ if err != nil {
+ openlog.Error(err.Error())
+ return err
+ }
+ kvOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(keyKV), etcdadpt.WithValue(bytes))
+ taskOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.TaskKey(kv.Domain, kv.Project, task.TaskID, task.Timestamp)), etcdadpt.WithValue(taskBytes))
+ err = etcdadpt.Txn(ctx, []etcdadpt.OpOptions{kvOpPut, taskOpPut})
+ if err != nil {
+ openlog.Error("update error", openlog.WithTags(openlog.Tags{
+ "err": err.Error(),
+ "kv": kv,
+ "task": task,
+ }))
+ return err
+ }
+ return nil
+}
+
+func (s *Dao) findOneAndDeleteWithTask(ctx context.Context, kvID, project, domain string) (*model.KVDoc, error) {
+ kvKey := key.KV(domain, project, kvID)
+ kvDoc := model.KVDoc{}
+ kvResp, err := etcdadpt.Get(ctx, kvKey)
+ if err != nil {
+ openlog.Error(err.Error())
+ return nil, err
+ }
+ if kvResp == nil {
+ return nil, datasource.ErrKeyNotExists
+ }
+ err = json.Unmarshal(kvResp.Value, &kvDoc)
+ if err != nil {
+ openlog.Error("fail to unmarshal kv" + err.Error())
+ return nil, err
+ }
+ // create task
+ task, err := datasource.NewTask(datasource.DeleteAction, datasource.ConfigResource)
+ if err != nil {
+ openlog.Error("fail to create task" + err.Error())
+ return nil, err
+ }
+
+ task.Data = kvDoc
+ taskBytes, err := json.Marshal(task)
+ if err != nil {
+ openlog.Error("fail to marshal task" + err.Error())
+ return nil, err
+ }
+ // create tombstone
Review comment:
减少行间注释，增加函数注释，或者抽取函数，可以参考单一职责设计原则
##########
File path: server/datasource/etcd/kv/kv_dao.go
##########
@@ -60,17 +69,24 @@ func (s *Dao) Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
}
//Update update key value
-func (s *Dao) Update(ctx context.Context, kv *model.KVDoc) error {
- keyKv := key.KV(kv.Domain, kv.Project, kv.ID)
- resp, err := etcdadpt.Get(ctx, keyKv)
+func (s *Dao) Update(ctx context.Context, kv *model.KVDoc, options ...datasource.CUDOption) error {
+ opts := datasource.NewDefaultCUDOpts()
+ for _, op := range options {
+ op(&opts)
+ }
+ // if syncEnable is true, will create kv with task
+ if opts.SyncEnable {
+ return s.updateWithTask(ctx, kv)
Review comment:
这里必须是createWithTask
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757186394
##########
File path: server/datasource/etcd/kv/kv_dao.go
##########
@@ -60,17 +69,24 @@ func (s *Dao) Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
}
//Update update key value
-func (s *Dao) Update(ctx context.Context, kv *model.KVDoc) error {
- keyKv := key.KV(kv.Domain, kv.Project, kv.ID)
- resp, err := etcdadpt.Get(ctx, keyKv)
+func (s *Dao) Update(ctx context.Context, kv *model.KVDoc, options ...datasource.CUDOption) error {
+ opts := datasource.NewDefaultCUDOpts()
+ for _, op := range options {
+ op(&opts)
+ }
+ // if syncEnable is true, will create kv with task
+ if opts.SyncEnable {
+ return s.updateWithTask(ctx, kv)
Review comment:
这里必须是createWithTask
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] robotLJW commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757751270
##########
File path: server/datasource/dao.go
##########
@@ -70,13 +74,14 @@ func GetBroker() Broker {
//KVDao provide api of KV entity
type KVDao interface {
// Create Update List are usually for admin console
- Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
- Update(ctx context.Context, kv *model.KVDoc) error
+ Create(ctx context.Context, kv *model.KVDoc, options ...WriteOption) (*model.KVDoc, error)
Review comment:
这边对sync的用例验证还是放到api层
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] robotLJW commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757750246
##########
File path: deployments/docker/mongo-compose.yaml
##########
@@ -0,0 +1,32 @@
+## ---------------------------------------------------------------------------
+## Licensed to the Apache Software Foundation (ASF) under one or more
+## contributor license agreements. See the NOTICE file distributed with
+## this work for additional information regarding copyright ownership.
+## The ASF licenses this file to You under the Apache License, Version 2.0
+## (the "License"); you may not use this file except in compliance with
+## the License. You may obtain a copy of the License at
+##
+## http://www.apache.org/licenses/LICENSE-2.0
+##
+## Unless required by applicable law or agreed to in writing, software
+## distributed under the License is distributed on an "AS IS" BASIS,
+## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+## See the License for the specific language governing permissions and
+## limitations under the License.
+## ---------------------------------------------------------------------------
+version: '3.3'
+services:
+ mongo:
+ image: mongo:4.2
+ command: mongod --replSet my-replica-set --port 27017 --bind_ip_all
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757185948
##########
File path: server/datasource/etcd/kv/kv_dao_trans.go
##########
@@ -0,0 +1,239 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package kv
+
+import (
+ "context"
+ "encoding/json"
+
+ "github.com/go-chassis/cari/sync"
+ "github.com/go-chassis/openlog"
+ "github.com/little-cui/etcdadpt"
+
+ "github.com/apache/servicecomb-kie/pkg/model"
+ "github.com/apache/servicecomb-kie/server/datasource"
+ "github.com/apache/servicecomb-kie/server/datasource/etcd/key"
+)
+
+func (s *Dao) createWithTask(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error) {
+ kvBytes, err := json.Marshal(kv)
+ if err != nil {
+ openlog.Error("marshal kv ")
+ return nil, err
+ }
+ task, err := datasource.NewTask(datasource.CreateAction, datasource.ConfigResource)
+ if err != nil {
+ openlog.Error("fail to create task" + err.Error())
+ return nil, err
+ }
+ task.Data = kv
+ taskBytes, err := json.Marshal(task)
+ if err != nil {
+ openlog.Error("fail to marshal task ")
+ return nil, err
+ }
+ kvOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.KV(kv.Domain, kv.Project, kv.ID)), etcdadpt.WithValue(kvBytes))
+ taskOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.TaskKey(kv.Domain, kv.Project, task.TaskID, task.Timestamp)), etcdadpt.WithValue(taskBytes))
+ err = etcdadpt.Txn(ctx, []etcdadpt.OpOptions{kvOpPut, taskOpPut})
+ if err != nil {
+ openlog.Error("create error", openlog.WithTags(openlog.Tags{
+ "err": err.Error(),
+ "kv": kv,
+ "task": task,
+ }))
+ return nil, err
+ }
+ return kv, nil
+}
+
+func (s *Dao) updateWithTask(ctx context.Context, kv *model.KVDoc) error {
Review comment:
update应该发生在任务处理的过程中，而且只能更新状态字段。
北向接口接受的所有写操作，都是不断地在创建新task，而不是针对通过一个业务实体，根据创建和更新操作对同一个task做更新
这是比较严重的业务逻辑错误
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] little-cui commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
little-cui commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757724988
##########
File path: server/datasource/dao.go
##########
@@ -70,13 +74,14 @@ func GetBroker() Broker {
//KVDao provide api of KV entity
type KVDao interface {
// Create Update List are usually for admin console
- Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
- Update(ctx context.Context, kv *model.KVDoc) error
+ Create(ctx context.Context, kv *model.KVDoc, options ...WriteOption) (*model.KVDoc, error)
Review comment:
请补充UT：
1、使用WriteOption = true
2、通过etcdadpt验证数据
##########
File path: server/service/kv/kv_svc_test.go
##########
@@ -178,3 +180,93 @@ func TestService_Delete(t *testing.T) {
assert.NoError(t, err)
})
}
+
+// Test the situation after the synchronization is turned on
+func TestSync(t *testing.T) {
+
+ t.Run("Transactional write configuration and generate synchronization tasks", func(t *testing.T) {
Review comment:
这些用例没有准确的断言，没有意义
##########
File path: server/service/kv/kv_svc_test.go
##########
@@ -178,3 +180,93 @@ func TestService_Delete(t *testing.T) {
assert.NoError(t, err)
})
}
+
+// Test the situation after the synchronization is turned on
+func TestSync(t *testing.T) {
+
+ t.Run("Transactional write configuration and generate synchronization tasks", func(t *testing.T) {
+ // turn on the synchronization switch
+ kcfg.Configurations.Sync.Enabled = true
+ var syncKVIDOne, syncKVIDTwo, syncKVIDThree string
+ // create three kvDoc
+ t.Run("create kv one,with labels app and service, should pass", func(t *testing.T) {
+ result, err := kvsvc.Create(context.TODO(), &model.KVDoc{
+ Key: "one",
+ Value: "1",
+ Status: common.StatusEnabled,
+ Labels: map[string]string{
+ "app": "one",
+ "service": "oneService",
+ },
+ Domain: domain,
+ Project: project,
+ })
+ assert.Nil(t, err)
+ assert.NotEmpty(t, result.ID)
+ assert.Equal(t, "1", result.Value)
+ syncKVIDOne = result.ID
+ })
+
+ t.Run("create kv two,with labels app and service, should pass", func(t *testing.T) {
+ result, err := kvsvc.Create(context.TODO(), &model.KVDoc{
+ Key: "two",
+ Value: "2",
+ Status: common.StatusEnabled,
+ Labels: map[string]string{
+ "app": "two",
+ "service": "twoService",
+ },
+ Domain: domain,
+ Project: project,
+ })
+ assert.Nil(t, err)
+ assert.NotEmpty(t, result.ID)
+ assert.Equal(t, "2", result.Value)
+ syncKVIDTwo = result.ID
+ })
+
+ t.Run("create kv three,with labels app and service, should pass", func(t *testing.T) {
+ result, err := kvsvc.Create(context.TODO(), &model.KVDoc{
+ Key: "three",
+ Value: "3",
+ Status: common.StatusEnabled,
+ Labels: map[string]string{
+ "app": "three",
+ "service": "threeService",
+ },
+ Domain: domain,
+ Project: project,
+ })
+ assert.Nil(t, err)
+ assert.NotEmpty(t, result.ID)
+ assert.Equal(t, "3", result.Value)
+ syncKVIDThree = result.ID
+ })
+
+ // update syncKVIDOne
+ t.Run("update kv by kvID, should pass", func(t *testing.T) {
+ result, err := kvsvc.Update(context.TODO(), &model.UpdateKVRequest{
+ ID: syncKVIDOne,
+ Value: "one",
+ Domain: domain,
+ Project: project,
+ })
+ assert.NoError(t, err)
+ assert.Equal(t, "one", result.Value)
+ })
+ // delete syncKVIDOne
+ t.Run("delete kv by kvID, should pass", func(t *testing.T) {
+ _, err := kvsvc.FindOneAndDelete(context.TODO(), syncKVIDOne, project, domain)
+ assert.NoError(t, err)
+ })
+
+ // delete syncKVIDTwo & syncKVIDThree
+ t.Run("delete kvs by array of kvID, should pass", func(t *testing.T) {
+ _, err := kvsvc.FindManyAndDelete(context.TODO(), []string{syncKVIDTwo, syncKVIDThree}, project, domain)
+ assert.NoError(t, err)
+ })
+
+ // stop sync
+ kcfg.Configurations.Sync.Enabled = false
Review comment:
使用defer即可
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] robotLJW commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r758014543
##########
File path: server/service/kv/kv_svc.go
##########
@@ -67,7 +68,7 @@ func ListKV(ctx context.Context, request *model.ListKVRequest) (int64, *model.KV
//Create get latest revision from history
//and increase revision of label
//and insert key
-func Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, *errsvc.Error) {
+func Create(ctx context.Context, kv *model.KVDoc, syncEnabled bool) (*model.KVDoc, *errsvc.Error) {
Review comment:
已经修改成option
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] robotLJW commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r756753157
##########
File path: server/service/kv/kv_svc.go
##########
@@ -250,9 +312,26 @@ func Update(ctx context.Context, kv *model.UpdateKVRequest) (*model.KVDoc, error
}
func FindOneAndDelete(ctx context.Context, kvID string, project, domain string) (*model.KVDoc, error) {
- kv, err := datasource.GetBroker().GetKVDao().FindOneAndDelete(ctx, kvID, project, domain)
- if err != nil {
- return nil, err
+ var kv *model.KVDoc
+ var err error
+ if cfg.GetSync().Enabled {
Review comment:
使用了CUDOption
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757180783
##########
File path: server/service/kv/kv_svc_test.go
##########
@@ -178,3 +180,90 @@ func TestService_Delete(t *testing.T) {
assert.NoError(t, err)
})
}
+
+// Test the situation after the synchronization is turned on
+func TestSync(t *testing.T) {
+ // start syncer
+ kcfg.Configurations.Sync.Enabled = true
+ var syncKVIDOne, syncKVIDTwo, syncKVIDThree string
+ // create three kvDoc
+ t.Run("create kv one,with labels app and service, should pass", func(t *testing.T) {
Review comment:
可以用嵌套的t.Run 减少维护的重复name
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757183935
##########
File path: server/datasource/dao.go
##########
@@ -70,13 +77,14 @@ func GetBroker() Broker {
//KVDao provide api of KV entity
type KVDao interface {
// Create Update List are usually for admin console
- Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
- Update(ctx context.Context, kv *model.KVDoc) error
+ Create(ctx context.Context, kv *model.KVDoc, options ...CUDOption) (*model.KVDoc, error)
Review comment:
嗯 我想错了，那么应该叫WriteOptions，或者叫DataOptions
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] little-cui commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
little-cui commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757724874
##########
File path: server/service/kv/kv_svc_test.go
##########
@@ -178,3 +180,93 @@ func TestService_Delete(t *testing.T) {
assert.NoError(t, err)
})
}
+
+// Test the situation after the synchronization is turned on
+func TestSync(t *testing.T) {
+
+ t.Run("Transactional write configuration and generate synchronization tasks", func(t *testing.T) {
Review comment:
这些用例没有准确的断言，与非Sync=true场景并行，不严谨
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] little-cui merged pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
little-cui merged pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757181290
##########
File path: server/datasource/dao.go
##########
@@ -70,13 +77,14 @@ func GetBroker() Broker {
//KVDao provide api of KV entity
type KVDao interface {
// Create Update List are usually for admin console
- Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
- Update(ctx context.Context, kv *model.KVDoc) error
+ Create(ctx context.Context, kv *model.KVDoc, options ...CUDOption) (*model.KVDoc, error)
Review comment:
应该是CRUD
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757176321
##########
File path: .github/workflows/mongo_storage.yml
##########
@@ -13,10 +13,7 @@ jobs:
uses: actions/checkout@v1
- name: UT
run: |
- cd build
- time bash build_docker.sh
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757358142
##########
File path: server/datasource/etcd/kv/kv_dao.go
##########
@@ -59,42 +56,98 @@ func (s *Dao) Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
return kv, nil
}
+func create(ctx context.Context, kv *model.KVDoc, options ...datasource.WriteOption) (bool, error) {
+ opts := datasource.NewWriteOptions(options...)
Review comment:
60-65抽离到外边的函数里就可以了
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] robotLJW commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r758087726
##########
File path: server/datasource/dao.go
##########
@@ -70,13 +74,14 @@ func GetBroker() Broker {
//KVDao provide api of KV entity
type KVDao interface {
// Create Update List are usually for admin console
- Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
- Update(ctx context.Context, kv *model.KVDoc) error
+ Create(ctx context.Context, kv *model.KVDoc, options ...WriteOption) (*model.KVDoc, error)
Review comment:
测试用例后面再补充，不好再service层加上开关参数，后面修改这容易将这个参数遗漏，还是需要 dao 层做验证，但是目前对task和tombstone的dao层没有写，无法做验证，后续补充sync的测试用例
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] robotLJW commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757751176
##########
File path: .github/workflows/mongo_storage.yml
##########
@@ -13,10 +13,7 @@ jobs:
uses: actions/checkout@v1
- name: UT
run: |
- cd build
Review comment:
已经另起 task 用于验证 build
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] little-cui commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
little-cui commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r758136310
##########
File path: deployments/db.js
##########
@@ -110,8 +115,11 @@ db.createCollection( "polling_detail", {
bsonType: "number"
}
}
- } }
-} );
+ }
+ }
+});
+db.createCollection("task");
+db.createCollection("tombstone");
Review comment:
不需要在这里加，应该在独立的module中做
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] robotLJW commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757750296
##########
File path: server/service/kv/kv_svc_test.go
##########
@@ -178,3 +180,93 @@ func TestService_Delete(t *testing.T) {
assert.NoError(t, err)
})
}
+
+// Test the situation after the synchronization is turned on
+func TestSync(t *testing.T) {
+
+ t.Run("Transactional write configuration and generate synchronization tasks", func(t *testing.T) {
Review comment:
这边修改到api层，传参进行修改
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757261257
##########
File path: server/datasource/etcd/kv/kv_dao_trans.go
##########
@@ -0,0 +1,254 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package kv
+
+import (
+ "context"
+ "encoding/json"
+
+ "github.com/go-chassis/cari/sync"
+ "github.com/go-chassis/openlog"
+ "github.com/little-cui/etcdadpt"
+
+ "github.com/apache/servicecomb-kie/pkg/model"
+ "github.com/apache/servicecomb-kie/server/datasource"
+ "github.com/apache/servicecomb-kie/server/datasource/etcd/key"
+)
+
+// CreateWithTask is to start transaction when creating KV, will create task in a transaction operation
+func (s *Dao) CreateWithTask(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error) {
+ kvBytes, err := json.Marshal(kv)
+ if err != nil {
+ openlog.Error("marshal kv ")
+ return nil, err
+ }
+ task, err := datasource.NewTask(sync.CreateAction, datasource.ConfigResource)
+ if err != nil {
+ openlog.Error("fail to create task" + err.Error())
+ return nil, err
+ }
+ task.Data = kv
+ taskBytes, err := json.Marshal(task)
+ if err != nil {
+ openlog.Error("fail to marshal task ")
+ return nil, err
+ }
+ kvOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.KV(kv.Domain, kv.Project, kv.ID)), etcdadpt.WithValue(kvBytes))
+ taskOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.TaskKey(kv.Domain, kv.Project, task.TaskID, task.Timestamp)), etcdadpt.WithValue(taskBytes))
+ err = etcdadpt.Txn(ctx, []etcdadpt.OpOptions{kvOpPut, taskOpPut})
Review comment:
其实区别应该就只是这一行上下的部分代码时独立的分支
##########
File path: server/datasource/etcd/kv/kv_dao_trans.go
##########
@@ -0,0 +1,254 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package kv
+
+import (
+ "context"
+ "encoding/json"
+
+ "github.com/go-chassis/cari/sync"
+ "github.com/go-chassis/openlog"
+ "github.com/little-cui/etcdadpt"
+
+ "github.com/apache/servicecomb-kie/pkg/model"
+ "github.com/apache/servicecomb-kie/server/datasource"
+ "github.com/apache/servicecomb-kie/server/datasource/etcd/key"
+)
+
+// CreateWithTask is to start transaction when creating KV, will create task in a transaction operation
+func (s *Dao) CreateWithTask(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error) {
+ kvBytes, err := json.Marshal(kv)
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757359511
##########
File path: server/datasource/etcd/kv/kv_dao.go
##########
@@ -59,42 +56,98 @@ func (s *Dao) Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
return kv, nil
}
+func create(ctx context.Context, kv *model.KVDoc, options ...datasource.WriteOption) (bool, error) {
+ opts := datasource.NewWriteOptions(options...)
+ kvBytes, err := json.Marshal(kv)
+ if err != nil {
+ openlog.Error("fail to marshal kv " + err.Error())
+ return false, err
+ }
+ // if syncEnable is true, will create task in a transaction operation
+ if opts.SyncEnable {
+ task, err := datasource.NewTask(sync.CreateAction, datasource.ConfigResource)
+ if err != nil {
+ openlog.Error("fail to create task" + err.Error())
+ return false, err
+ }
+ task.Data = kv
+ taskBytes, err := json.Marshal(task)
+ if err != nil {
+ openlog.Error("fail to marshal task ")
+ return false, err
+ }
+ kvOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.KV(kv.Domain, kv.Project, kv.ID)), etcdadpt.WithValue(kvBytes))
+ taskOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.TaskKey(kv.Domain, kv.Project, task.TaskID, task.Timestamp)), etcdadpt.WithValue(taskBytes))
+ cmpOpts := []etcdadpt.CmpOptions{
+ etcdadpt.OpCmp(etcdadpt.CmpCreateRev(kvOpPut.Key), etcdadpt.CmpEqual, 0),
+ etcdadpt.OpCmp(etcdadpt.CmpCreateRev(taskOpPut.Key), etcdadpt.CmpEqual, 0),
+ }
+ resp, err := etcdadpt.TxnWithCmp(ctx, []etcdadpt.OpOptions{kvOpPut, taskOpPut}, cmpOpts, nil)
+ if err != nil {
+ return false, err
+ }
+ return resp.Succeeded, nil
+ }
+ return etcdadpt.InsertBytes(ctx, key.KV(kv.Domain, kv.Project, kv.ID), kvBytes)
+}
+
//Update update key value
-func (s *Dao) Update(ctx context.Context, kv *model.KVDoc) error {
- keyKv := key.KV(kv.Domain, kv.Project, kv.ID)
- resp, err := etcdadpt.Get(ctx, keyKv)
+func (s *Dao) Update(ctx context.Context, kv *model.KVDoc, options ...datasource.WriteOption) error {
+ keyKV := key.KV(kv.Domain, kv.Project, kv.ID)
+ resp, err := etcdadpt.Get(ctx, keyKV)
if err != nil {
openlog.Error(err.Error())
return err
}
if resp == nil {
- return datasource.ErrRecordNotExists
+ return datasource.ErrKeyNotExists
}
-
- var old model.KVDoc
- err = json.Unmarshal(resp.Value, &old)
+ var oldKV model.KVDoc
+ err = json.Unmarshal(resp.Value, &oldKV)
if err != nil {
openlog.Error(err.Error())
return err
}
- old.LabelFormat = kv.LabelFormat
- old.Value = kv.Value
- old.Status = kv.Status
- old.Checker = kv.Checker
- old.UpdateTime = kv.UpdateTime
- old.UpdateRevision = kv.UpdateRevision
-
- bytes, err := json.Marshal(old)
+ oldKV.LabelFormat = kv.LabelFormat
+ oldKV.Value = kv.Value
+ oldKV.Status = kv.Status
+ oldKV.Checker = kv.Checker
+ oldKV.UpdateTime = kv.UpdateTime
+ oldKV.UpdateRevision = kv.UpdateRevision
+ err = update(ctx, &oldKV, options...)
if err != nil {
openlog.Error(err.Error())
return err
}
- err = etcdadpt.PutBytes(ctx, keyKv, bytes)
+ return nil
+}
+
+func update(ctx context.Context, kv *model.KVDoc, options ...datasource.WriteOption) error {
+ opts := datasource.NewWriteOptions(options...)
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] robotLJW commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r756541586
##########
File path: .github/workflows/mongo_storage.yml
##########
@@ -13,11 +13,8 @@ jobs:
uses: actions/checkout@v1
- name: UT
run: |
- cd build
- time bash build_docker.sh
- cd ../
- sudo docker-compose -f ./deployments/docker/docker-compose.yaml up -d
- sleep 20
+ sudo docker-compose -f ./deployments/docker/mongo-compose.yaml up -d
+ sleep 15
export TEST_DB_KIND=mongo
- export TEST_DB_URI=mongodb://kie:123@127.0.0.1:27017/kie
+ export TEST_DB_URI=mongodb://kie:123@127.0.0.1:30001/kie
Review comment:
主要是因为副本集以27017端口起不来
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] robotLJW commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757182016
##########
File path: server/datasource/dao.go
##########
@@ -70,13 +77,14 @@ func GetBroker() Broker {
//KVDao provide api of KV entity
type KVDao interface {
// Create Update List are usually for admin console
- Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
- Update(ctx context.Context, kv *model.KVDoc) error
+ Create(ctx context.Context, kv *model.KVDoc, options ...CUDOption) (*model.KVDoc, error)
Review comment:
这边考虑R 查找不需要写人物就能加上去了
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] tianxiaoliang commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757185948
##########
File path: server/datasource/etcd/kv/kv_dao_trans.go
##########
@@ -0,0 +1,239 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package kv
+
+import (
+ "context"
+ "encoding/json"
+
+ "github.com/go-chassis/cari/sync"
+ "github.com/go-chassis/openlog"
+ "github.com/little-cui/etcdadpt"
+
+ "github.com/apache/servicecomb-kie/pkg/model"
+ "github.com/apache/servicecomb-kie/server/datasource"
+ "github.com/apache/servicecomb-kie/server/datasource/etcd/key"
+)
+
+func (s *Dao) createWithTask(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error) {
+ kvBytes, err := json.Marshal(kv)
+ if err != nil {
+ openlog.Error("marshal kv ")
+ return nil, err
+ }
+ task, err := datasource.NewTask(datasource.CreateAction, datasource.ConfigResource)
+ if err != nil {
+ openlog.Error("fail to create task" + err.Error())
+ return nil, err
+ }
+ task.Data = kv
+ taskBytes, err := json.Marshal(task)
+ if err != nil {
+ openlog.Error("fail to marshal task ")
+ return nil, err
+ }
+ kvOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.KV(kv.Domain, kv.Project, kv.ID)), etcdadpt.WithValue(kvBytes))
+ taskOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.TaskKey(kv.Domain, kv.Project, task.TaskID, task.Timestamp)), etcdadpt.WithValue(taskBytes))
+ err = etcdadpt.Txn(ctx, []etcdadpt.OpOptions{kvOpPut, taskOpPut})
+ if err != nil {
+ openlog.Error("create error", openlog.WithTags(openlog.Tags{
+ "err": err.Error(),
+ "kv": kv,
+ "task": task,
+ }))
+ return nil, err
+ }
+ return kv, nil
+}
+
+func (s *Dao) updateWithTask(ctx context.Context, kv *model.KVDoc) error {
Review comment:
update应该发生在任务处理的过程中，而且只能更新状态字段。
北向接口接受的所有写操作，都是不断地在创建新task，而不是针对通过一个业务实体，根据创建和更新操作对同一个task做更新
这是比较严重的业务逻辑错误
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [servicecomb-kie] robotLJW commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on
Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r757253042
##########
File path: server/datasource/dao.go
##########
@@ -38,14 +41,18 @@ var (
ErrKeyNotExists = errors.New("can not find any key value")
ErrRecordNotExists = errors.New("can not find any polling data")
ErrRevisionNotExist = errors.New("revision does not exist")
- ErrAliasNotGiven = errors.New("label alias not given")
ErrKVAlreadyExists = errors.New("kv already exists")
ErrTooMany = errors.New("key with labels should be only one")
)
const (
DefaultValueType = "text"
MaxHistoryNum = 100
+
+ CreateAction = "create"
Review comment:
已经是用cari包内
--
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.
To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org