robocanic commented on code in PR #1477:
URL: https://github.com/apache/dubbo-admin/pull/1477#discussion_r3407613474


##########
pkg/config/app/admin.go:
##########
@@ -51,6 +52,8 @@ type AdminConfig struct {
        Engine *engine.Config `json:"engine" yaml:"engine"`
        // EventBus configuration
        EventBus *eventbus.Config `json:"eventBus,omitempty" 
yaml:"eventBus,omitempty"`
+       // Versioning configuration for governor-managed traffic rules.
+       Versioning *versioning.Config `json:"versioning,omitempty" 
yaml:"versioning,omitempty"`

Review Comment:
   Suggestion: 不要叫Versioning,这个名字太宽泛了,用`Rule`来表示路由规则领域的配置,会更清晰和规范



##########
pkg/config/app/admin.go:
##########
@@ -65,10 +68,12 @@ var DefaultAdminConfig = func() AdminConfig {
                Diagnostics:   diagnostics.DefaultDiagnosticsConfig(),
                Console:       console.DefaultConsoleConfig(),
                EventBus:      &eventBusCfg,
+               Versioning:    versioning.Default(),
        }
 }
 
-func (c AdminConfig) Sanitize() {
+func (c *AdminConfig) Sanitize() {
+       c.ensureDefaults()

Review Comment:
   Question: 为什么要加这一行?



##########
pkg/core/store/store.go:
##########
@@ -34,6 +34,8 @@ import (
 // ResourceStore expanded the interface of cache.Indexer and cache.Store
 type ResourceStore interface {
        Indexer
+       // ListResources lists all resources in this store with error 
propagation.
+       ListResources() ([]model.Resource, error)

Review Comment:
   危险操作,不应该在接口层暴露这种全扫的操作



##########
pkg/config/app/admin.go:
##########
@@ -171,9 +182,42 @@ func (c AdminConfig) Validate() error {
        } else if err := c.EventBus.Validate(); err != nil {
                return bizerror.Wrap(err, bizerror.ConfigError, "event bus 
config validation failed")
        }
+       if c.Versioning == nil {
+               c.Versioning = versioning.Default()
+       } else if err := c.Versioning.Validate(); err != nil {
+               return bizerror.Wrap(err, bizerror.ConfigError, "versioning 
config validation failed")
+       }
        return nil
 }
 
+func (c *AdminConfig) ensureDefaults() {

Review Comment:
   Question: 为什么要ensureDefaults?



##########
pkg/core/versioning/store.go:
##########


Review Comment:
   
Suggestion:为什么不将Version作为一种Resource,复用现有的ResourceStore,ResourceManager链路呢?写了很多重复代码



##########
pkg/core/versioning/service.go:
##########
@@ -0,0 +1,311 @@
+/*
+ * 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 versioning
+
+import (
+       "encoding/json"
+       "strconv"
+       "strings"
+       "time"
+
+       meshproto "github.com/apache/dubbo-admin/api/mesh/v1alpha1"
+       "github.com/apache/dubbo-admin/pkg/common/bizerror"
+       meshresource 
"github.com/apache/dubbo-admin/pkg/core/resource/apis/mesh/v1alpha1"
+       coremodel "github.com/apache/dubbo-admin/pkg/core/resource/model"
+       "google.golang.org/protobuf/encoding/protojson"
+)
+
+type Service interface {

Review Comment:
   Suggestion:Golang中不提倡这种宽Service Interface接口,多这一层没有意义



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to