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/06/04 10:30:25 UTC
[GitHub] [servicecomb-service-center] tianxiaoliang opened a new pull request #1040: refactor: must eliminate god class DataSource
tianxiaoliang opened a new pull request #1040:
URL: https://github.com/apache/servicecomb-service-center/pull/1040
Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
- [ ] Each commit in the pull request should have a meaningful subject line and body.
- [ ] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Run `go build` `go test` `go fmt` `go vet` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
- [ ] Never comment source code, delete it.
- [ ] UT should has "context, subject, expected result" result as test case name, when you call t.Run().
---
--
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
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a change in pull request #1040: refactor: must eliminate god class DataSource
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #1040:
URL: https://github.com/apache/servicecomb-service-center/pull/1040#discussion_r647887559
##########
File path: datasource/datasource.go
##########
@@ -17,12 +17,27 @@
package datasource
+import (
+ "context"
+)
+
// DataSource is the DAO layer
type DataSource interface {
- SystemManager
- AccountManager
- RoleManager
- DependencyManager
- MetadataManager
- SCManager
+ SystemManager() SystemManager
+ AccountManager() AccountManager
+ RoleManager() RoleManager
+ DependencyManager() DependencyManager
+ MetadataManager() MetadataManager
+ SCManager() SCManager
+}
+
+type DataEvent struct {
+ Type string
+ Data interface{}
+}
+type Watcher interface {
+ Result() <-chan *DataEvent
+}
+type DataWatcher interface {
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
[GitHub] [servicecomb-service-center] tianxiaoliang commented on a change in pull request #1040: refactor: must eliminate god class DataSource
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #1040:
URL: https://github.com/apache/servicecomb-service-center/pull/1040#discussion_r647888287
##########
File path: datasource/etcd/version.go
##########
@@ -50,43 +48,8 @@ func (ds *DataSource) LoadServerVersion(ctx context.Context) error {
return nil
}
-func (ds *DataSource) UpgradeServerVersion(ctx context.Context) error {
- bytes, err := json.Marshal(config.Server)
- if err != nil {
- return err
- }
- _, err = client.Instance().Do(ctx,
- client.PUT, client.WithStrKey(path.GetServerInfoKey()), client.WithValue(bytes))
- if err != nil {
- return err
- }
- return nil
-}
-
-func (ds *DataSource) UpgradeVersion(ctx context.Context) error {
- lock, err := mux.Lock(mux.GlobalLock)
-
- if err != nil {
- log.Errorf(err, "wait for server ready failed")
- return err
- }
- if ds.needUpgrade(ctx) {
- config.Server.Version = version.Ver().Version
-
- if err := ds.UpgradeServerVersion(ctx); err != nil {
- log.Errorf(err, "upgrade server version failed")
- os.Exit(1)
- }
- }
- err = lock.Unlock()
- if err != nil {
- log.Error("", err)
- }
- return err
-}
-
-func (ds *DataSource) needUpgrade(ctx context.Context) bool {
- err := ds.LoadServerVersion(ctx)
+func needUpgrade(ctx context.Context) bool {
+ err := loadServerVersion(ctx)
Review comment:
没有删掉逻辑，而是这个ds指针放在这里没有任何意义
--
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
[GitHub] [servicecomb-service-center] robotLJW merged pull request #1040: refactor: must eliminate god class DataSource
Posted by GitBox <gi...@apache.org>.
robotLJW merged pull request #1040:
URL: https://github.com/apache/servicecomb-service-center/pull/1040
--
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
[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #1040: refactor: must eliminate god class DataSource
Posted by GitBox <gi...@apache.org>.
little-cui commented on a change in pull request #1040:
URL: https://github.com/apache/servicecomb-service-center/pull/1040#discussion_r647154164
##########
File path: datasource/datasource.go
##########
@@ -17,12 +17,27 @@
package datasource
+import (
+ "context"
+)
+
// DataSource is the DAO layer
type DataSource interface {
- SystemManager
- AccountManager
- RoleManager
- DependencyManager
- MetadataManager
- SCManager
+ SystemManager() SystemManager
+ AccountManager() AccountManager
+ RoleManager() RoleManager
+ DependencyManager() DependencyManager
+ MetadataManager() MetadataManager
+ SCManager() SCManager
+}
+
+type DataEvent struct {
+ Type string
+ Data interface{}
+}
+type Watcher interface {
+ Result() <-chan *DataEvent
+}
+type DataWatcher interface {
Review comment:
事件监听的接口应该独立一个文件或pkg
##########
File path: datasource/etcd/version.go
##########
@@ -50,43 +48,8 @@ func (ds *DataSource) LoadServerVersion(ctx context.Context) error {
return nil
}
-func (ds *DataSource) UpgradeServerVersion(ctx context.Context) error {
- bytes, err := json.Marshal(config.Server)
- if err != nil {
- return err
- }
- _, err = client.Instance().Do(ctx,
- client.PUT, client.WithStrKey(path.GetServerInfoKey()), client.WithValue(bytes))
- if err != nil {
- return err
- }
- return nil
-}
-
-func (ds *DataSource) UpgradeVersion(ctx context.Context) error {
- lock, err := mux.Lock(mux.GlobalLock)
-
- if err != nil {
- log.Errorf(err, "wait for server ready failed")
- return err
- }
- if ds.needUpgrade(ctx) {
- config.Server.Version = version.Ver().Version
-
- if err := ds.UpgradeServerVersion(ctx); err != nil {
- log.Errorf(err, "upgrade server version failed")
- os.Exit(1)
- }
- }
- err = lock.Unlock()
- if err != nil {
- log.Error("", err)
- }
- return err
-}
-
-func (ds *DataSource) needUpgrade(ctx context.Context) bool {
- err := ds.LoadServerVersion(ctx)
+func needUpgrade(ctx context.Context) bool {
+ err := loadServerVersion(ctx)
Review comment:
这部分逻辑删除的原因是什么？原有的设计是用于记录当前运行的sc版本跟之前运行的版本差异，提供给scctl等工具做迁移做判断使用
--
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