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

Support different storage #380

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

yvanu
Copy link

@yvanu yvanu commented Apr 23, 2024

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?


Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


@yvanu yvanu force-pushed the support-different-storage branch from c02ed27 to 314f6ee Compare April 24, 2024 06:00
config.yaml Outdated
log_format: json

db:
type: sqlite
Copy link
Contributor

Choose a reason for hiding this comment

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

冗余信息,只有 sqlite 而没有 mysql 就已经能说明要连接 sqlite 类型的数据库了

return err
}
type DbConfig struct {
Type string `yaml:"type"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Type 是冗余信息,mysql 和 sqlite 字段用指针

@@ -107,7 +108,7 @@ func (c *cluster) Create(ctx context.Context, req *types.CreateClusterRequest) e
}

var cs *client.ClusterSet
var txFunc db.TxFunc = func() (err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

var txFunc = func() error 去掉类型,没必要为它多写一个包

Copy link
Author

Choose a reason for hiding this comment

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

这个是之前既有的类型定义,在db包下。
新开一个包写它也是为了解决循环依赖

package iface

import (
"context"
Copy link
Contributor

Choose a reason for hiding this comment

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

原生包和三方包之间空行

@@ -0,0 +1,32 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

开源许可证在写完后,用 make licfmt 生成

Comment on lines 80 to 89
func (c *Config) Valid() (err error) {
if err = c.Default.Valid(); err != nil {
return
}
if err = c.Mysql.Valid(); err != nil {
return
}
return
func (c *Config) Valid() error {
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

要把自己负责的部分的校验逻辑加进来,而不是删掉原先已存在的校验逻辑

Comment on lines 29 to 33
type ShareDaoFactory interface {
Cluster() ClusterInterface
Tenant() TenantInterface
User() UserInterface
}

type shareDaoFactory struct {
db *gorm.DB
Cluster() iface.ClusterInterface
Tenant() iface.TenantInterface
User() iface.UserInterface
}
Copy link
Contributor

Choose a reason for hiding this comment

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

没必要另起文件,ClusterInterface TenantInterface UserInterface 就该在原来的文件中

Copy link
Author

Choose a reason for hiding this comment

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

这个是为了解决循环依赖的

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.

2 participants