Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Features register #19

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

githublaohu
Copy link

No description provided.

.vscode/launch.json Outdated Show resolved Hide resolved
conf/api_server.toml Outdated Show resolved Hide resolved
cache/naming/DEFAULT_GROUP@@atom_runtime_python_service Outdated Show resolved Hide resolved
conf/api_server.toml Outdated Show resolved Hide resolved
stateful/config_register.go Outdated Show resolved Hide resolved
stateful/config_register.go Outdated Show resolved Hide resolved
stateful/config_register.go Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
model/icluster_conf/pool_instance.go Outdated Show resolved Hide resolved
stateful/config.go Outdated Show resolved Hide resolved
storage/nacos/cluster_conf/pool_instance.go Outdated Show resolved Hide resolved
conf/api_server.toml Outdated Show resolved Hide resolved
docs/zh_cn/config_param.md Outdated Show resolved Hide resolved
endpoints/openapi_v1/bfe_pool/create.go Outdated Show resolved Hide resolved
endpoints/openapi_v1/bfe_pool/delete.go Outdated Show resolved Hide resolved
endpoints/openapi_v1/bfe_pool/one.go Outdated Show resolved Hide resolved
endpoints/openapi_v1/product_pool/create.go Outdated Show resolved Hide resolved
endpoints/openapi_v1/product_pool/delete.go Outdated Show resolved Hide resolved
endpoints/openapi_v1/bfe_pool/update.go Outdated Show resolved Hide resolved
model/icluster_conf/pool_instance.go Outdated Show resolved Hide resolved
storage/rdb/cluster_conf/pool_instance.go Outdated Show resolved Hide resolved
model/icluster_conf/cluster.go Outdated Show resolved Hide resolved
model/icluster_conf/cluster.go Outdated Show resolved Hide resolved
model/icluster_conf/pool.go Outdated Show resolved Hide resolved
model/icluster_conf/pool.go Outdated Show resolved Hide resolved
model/icluster_conf/pool.go Outdated Show resolved Hide resolved
storage/rdb/cluster_conf/pool_instance.go Outdated Show resolved Hide resolved
model/icluster_conf/pool.go Outdated Show resolved Hide resolved
model/icluster_conf/pool.go Show resolved Hide resolved
model/icluster_conf/pool.go Outdated Show resolved Hide resolved
model/icluster_conf/pool.go Show resolved Hide resolved
}

for typ := range type2InstancePoolList {
_, ok := pim.instancePoolStorages[typ]
for type2 := range type2PoolList {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type2 => type?
如果type是保留字,可以用typeName。
type2不知道是什么意思

}
}

rst := map[string]*InstancePool{}
for typ, pisList := range type2InstancePoolList {
storager := pim.instancePoolStorages[typ]
for type2, pisList := range type2PoolList {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同line 74

// CanDelete check whether pool can be deleted, Check Logic:
// 1. Not BFE Cluster Refer To
// 2. Not SubCluster Refer To
func (m *PoolManager) CanDelete(ctx context.Context, pool *Pool) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个函数的返回值有些问题。
从函数名字看,应该“逻辑判断型”的返回值。
建议返回值改为 (bool, error),第一个返回值返回true/false,第二个返回错误的信息

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个问题并没有答复或解决

return
}

func (m *PoolManager) FetchBFEPools(ctx context.Context) (list []*Pool, err error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个函数存在的意义是?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个问题没有答复呀

return
}

func poolNameJudger(productName string, poolName string) (realName string, err error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

poolNameJudger => poolNameGet

  1. *er一般是一个对象。这个函数显然只是一个操作
  2. 如果语义是judge/check,就没有“修改”的含义。改为Get后会好很多

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个问题也没有任何的解释和答复


type PoolManager struct {
storage PoolStorage
bfeClusterStorager ibasic.BFEClusterStorager
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storager => Storage
所有代码的都替换一下吧。
既然line 27已经是storage,那就统一风格

Copy link
Author

@githublaohu githublaohu Jun 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

本次设计里面没有编写BFEClusterStorager,是以前的代码


func (m *PoolManager) CreateProductPool(ctx context.Context, product *ibasic.Product, param *PoolParam, pool *InstancePool) (one *Pool, err error) {
var pN string
pN, err = poolNameJudger(product.Name, *param.Name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pN => poolName。
pN这样的缩写完全看不懂,是非常差的起名方式。
缩写还是要让人能看懂

model/icluster_conf/pool_manager.go Show resolved Hide resolved
var _ icluster_conf.InstancePoolStorage = &RDBInstancePoolStorage{}

func (rpps *RDBInstancePoolStorage) UpdateInstances(ctx context.Context, pool *icluster_conf.Pool,
pis *icluster_conf.InstancePool) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pis这个缩写并不合适,改为iPool?

return err
}

func (rpps *RDBInstancePoolStorage) BatchFetchInstances(ctx context.Context,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rpps不知道是什么的缩写

@@ -201,7 +192,7 @@ func (rpps *RDBPoolStorager) UpdatePool(ctx context.Context, oldData *icluster_c
return err
}

func (rpps *RDBPoolStorager) DeletePool(ctx context.Context, pool *icluster_conf.Pool) error {
func (rpps *RDBPoolStorage) DeletePool(ctx context.Context, pool *icluster_conf.Pool) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rpps => s, 或 rps


return rst, nil
}

func (rpps *RDBPoolStorager) UpdatePool(ctx context.Context, oldData *icluster_conf.Pool,
func (rpps *RDBPoolStorage) UpdatePool(ctx context.Context, oldData *icluster_conf.Pool,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rpps => s, 或 rps

}, nil
}

func (rpps *RDBPoolStorager) CreatePool(ctx context.Context, product *ibasic.Product,
func (rpps *RDBPoolStorage) CreatePool(ctx context.Context, product *ibasic.Product,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rpps => s, 或 rps


rst := map[string]*InstancePool{}
for type2, pisList := range type2PoolList {
storager := m.instancePoolStorages[type2]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

storager => storage

return rst, nil
}

func (m *InstancePoolManager) UpdateInstances(ctx context.Context, pool *Pool, pis *InstancePool) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pis => iPool

return
}

func (m *PoolManager) FetchBFEPools(ctx context.Context) (list []*Pool, err error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个问题没有答复呀

return
}

func poolNameJudger(productName string, poolName string) (realName string, err error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个问题也没有任何的解释和答复

// CanDelete check whether pool can be deleted, Check Logic:
// 1. Not BFE Cluster Refer To
// 2. Not SubCluster Refer To
func (m *PoolManager) CanDelete(ctx context.Context, pool *Pool) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个问题并没有答复或解决

@bumu
Copy link

bumu commented Apr 18, 2023

What's the progress

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants