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

feat(core): createInstance() accepts 3rd param cacheInstance (default true) #2208

Merged
merged 2 commits into from Aug 9, 2022

Conversation

waitingsong
Copy link
Member

No description provided.

@gitpod-io
Copy link

gitpod-io bot commented Aug 9, 2022

}
return client;
if (cacheInstance && clientName && client) {
this.dataSource.set(clientName, client);
Copy link
Member

Choose a reason for hiding this comment

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

这里有不缓存的需求?统一缓存可以做统一关闭和管理以及 manager 上的方法,不缓存是不是甚至都不需要走这里了。。

Copy link
Member Author

Choose a reason for hiding this comment

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

刚才就遇到这种场景了: task 任务,创建 sqlite 实例,生成 sqlite 数据库(随机文件名)然后上传 oss , 接下来客户端下载这个 db 文件更新本地信息。

Copy link
Member Author

Choose a reason for hiding this comment

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

db 文件名随机,并且还不能用同一个 sourceName 值(比如 'sqlite')。所以不能在配置文件中写配置信息(主要是 sourceName),而必须动态创建(并且不缓存)

Copy link
Member Author

Choose a reason for hiding this comment

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

通常场景(mysql, pg)数据库是个服务,长期提供服务。而我这儿的是要生成 包含数据的 sqlite 数据库用于分发。

Copy link
Member Author

Choose a reason for hiding this comment

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

不缓存是不是甚至都不需要走这里了

实际业务是需要连接 mysql 数据库,读取数据,写入到 sqlite 文件中。
如果不能走同一个入口,代码就不好管理。

Copy link
Member

Choose a reason for hiding this comment

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

噢,懂了。

@codecov-commenter
Copy link

codecov-commenter commented Aug 9, 2022

Codecov Report

Merging #2208 (e69644a) into main (94ddd69) will decrease coverage by 0.12%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2208      +/-   ##
==========================================
- Coverage   82.42%   82.29%   -0.13%     
==========================================
  Files         434      434              
  Lines       15211    15214       +3     
  Branches     3593     3595       +2     
==========================================
- Hits        12537    12520      -17     
- Misses       2446     2463      +17     
- Partials      228      231       +3     
Impacted Files Coverage Δ
packages/core/src/common/dataSourceManager.ts 100.00% <100.00%> (ø)
packages/oss/src/manager.ts 38.88% <0.00%> (-50.00%) ⬇️
packages/oss/src/configuration.ts 60.00% <0.00%> (-40.00%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

public async createInstance(
config,
clientName,
cacheInstance = true
Copy link
Member

Choose a reason for hiding this comment

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

感觉这里可能后面会有别的参数,写成 options 吧。

Copy link
Member Author

Choose a reason for hiding this comment

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

那我修改下

@waitingsong waitingsong force-pushed the datasource branch 3 times, most recently from f0bfcb6 to 05fe7e1 Compare August 9, 2022 09:20
```ts
interface CreateInstanceOptions {
  /**
   * @default true
   */
  cacheInstance?: boolean | undefined;
}
```
@czy88840616 czy88840616 merged commit a9149c2 into midwayjs:main Aug 9, 2022
@waitingsong waitingsong deleted the datasource branch August 9, 2022 12:36
@waitingsong
Copy link
Member Author

这样无论是静态配置文件的(自动)初始化,还是业务场景下的动态初始化,都可以把数据库初始化入口统一收口到 createInstance() 方法上。
不然,对于动态创建实例场景,很容易就会调用到抽象方法 createDataSource() 的自我实现上,
于是一会儿是 createInstance,一会是 createDataSource,口子一开就再也收不回来了。以后维护很麻烦。

@czy88840616
Copy link
Member

czy88840616 commented Aug 9, 2022

createDataSource 是protected的,继承之后也希望是 protected,另一方面,createInstance 和 createDataSource 差了一个 default 的配置,基本区别就是这样了。

@waitingsong
Copy link
Member Author

试了下,声明成 public 也行

@Provide()
@Scope(ScopeEnum.Singleton)
export class DbSourceManager extends DataSourceManager<Db> {

  /** 创建单个实例 */
  public async createDataSource(
    options: DbConfig,
    dataSourceName: SourceName,
  ): Promise<Db> {
  .....
 }

@czy88840616
Copy link
Member

是可以,这就是 developer 的自有行为了,这个和 serviceFactory 是一样的,一个用来创建纯的客户端(用来继承实现,不给用户),一个是用来合并 defaut配置透出给用户使用的方法

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

Successfully merging this pull request may close these issues.

None yet

3 participants