-
Notifications
You must be signed in to change notification settings - Fork 142
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
refactor(ava/advisor): init new advisor pipeline & built-in modules to plugin-based architecture #784
base: refactor-advisor-pipeline
Are you sure you want to change the base?
refactor(ava/advisor): init new advisor pipeline & built-in modules to plugin-based architecture #784
Changes from 1 commit
a6af04d
c35a204
a86960e
3d32447
85cc0ce
70b5899
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,10 +19,10 @@ import type { | |
Lint, | ||
AdvisorPipelineContext, | ||
PipelineStageType, | ||
PluginType, | ||
AdvisorPluginType, | ||
DataProcessorInput, | ||
DataProcessorOutput, | ||
ChartTypeRecommendInputParams, | ||
ChartTypeRecommendInput, | ||
ChartTypeRecommendOutput, | ||
VisualEncoderInput, | ||
VisualEncoderOutput, | ||
|
@@ -43,32 +43,36 @@ export class Advisor { | |
|
||
dataAnalyzer: BaseComponent<DataProcessorInput, DataProcessorOutput>; | ||
|
||
chartTypeRecommender: BaseComponent<ChartTypeRecommendInputParams, ChartTypeRecommendOutput>; | ||
chartTypeRecommender: BaseComponent<ChartTypeRecommendInput, ChartTypeRecommendOutput>; | ||
|
||
chartEncoder: BaseComponent<VisualEncoderInput, VisualEncoderOutput>; | ||
|
||
specGenerator: BaseComponent<SpecGeneratorInput, SpecGeneratorOutput>; | ||
|
||
context: AdvisorPipelineContext; | ||
|
||
plugins: PluginType[]; | ||
plugins: AdvisorPluginType[]; | ||
|
||
pipeline: Pipeline; | ||
|
||
constructor( | ||
config: AdvisorConfig = {}, | ||
custom: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 当前仅 constructor 输入 custom 不知道是否支持后续 实例动态注册? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 需要的,后续打算在 advisor 上增加 updateCkb, updateRuleBase, resigterComponents 等 api |
||
plugins?: PluginType[]; | ||
plugins?: AdvisorPluginType[]; | ||
components?: BaseComponent[]; | ||
/** extra info to pass through the pipeline | ||
* 额外透传到推荐 pipeline 中的业务信息 | ||
*/ | ||
extra?: Record<string, any>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 同理,extra 直接放 ctx 上? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 是放在 context 上的,这里是 constructor 时进行初始化传参 |
||
} = {} | ||
) { | ||
// init | ||
const { plugins, components, extra = {} } = custom; | ||
this.ckb = ckb(config.ckbCfg); | ||
this.ruleBase = processRuleCfg(config.ruleCfg); | ||
this.context = { advisor: this }; | ||
this.context = { advisor: this, extra }; | ||
this.initDefaultComponents(); | ||
const defaultComponents = [this.dataAnalyzer, this.chartTypeRecommender, this.chartEncoder, this.specGenerator]; | ||
const { plugins, components } = custom; | ||
this.plugins = plugins; | ||
this.pipeline = new Pipeline({ components: components ?? defaultComponents }); | ||
} | ||
|
@@ -83,11 +87,6 @@ export class Advisor { | |
this.specGenerator = new BaseComponent('specGenerate', { plugins: [specGeneratorPlugin], context: this.context }); | ||
} | ||
|
||
// todo 定义多个链路串并联的方法 | ||
// private definePipeline(components: BaseComponent[]) { | ||
// this.pipeline.components = components; | ||
// } | ||
|
||
// todo 暂时还在用旧链路,需要改造到新链路 | ||
advise(params: AdviseParams): Advice[] { | ||
const adviseResult = dataToAdvices({ adviseParams: params, ckb: this.ckb, ruleBase: this.ruleBase }); | ||
|
@@ -119,7 +118,7 @@ export class Advisor { | |
return lintResult; | ||
} | ||
|
||
registerPlugins(plugins: PluginType[]) { | ||
registerPlugins(plugins: AdvisorPluginType[]) { | ||
const stage2Components: Record<PipelineStageType, BaseComponent> = { | ||
dataAnalyze: this.dataAnalyzer, | ||
chartTypeRecommend: this.chartTypeRecommender, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ import type { MarkEncode } from './mark'; | |
export type PipelineStageType = 'dataAnalyze' | 'chartTypeRecommend' | 'encode' | 'specGenerate'; | ||
|
||
/** 基础插件接口定义 */ | ||
export interface PluginType<Input = any, Output = any> { | ||
export interface AdvisorPluginType<Input = any, Output = any> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 我有一个大胆的想法,如果 advice pipeline 是确定的,有几个 component 也是确定的。这条 pipeline 是不是都属于一个 ctx,那这条链路上的 input 和 output 其实都是从当前的 ctx 中获得,而不用区分?也就是说这里的 Input 和 Output 其实都是 ctx。况且看下面 stage 还有多个的情况。 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 意思是说,pipeline context 里加上 input 和 output 属性么,这两个属性根据当前执行的环节进行更新? 每个 component 和 plugin 用类型约束,是想说用户在自定义自己的 plugin 时,能够明确需要的类型,都放 pipeline 感觉类型不好约束了? |
||
/** 插件的唯一标识 */ | ||
name: string; | ||
/** 插件运行的阶段,用于指定插件在 pipeline 的哪个环节运行 * */ | ||
|
@@ -30,7 +30,7 @@ export type DataProcessorOutput = { | |
dataProps: BasicDataPropertyForAdvice[]; | ||
}; | ||
|
||
export type ChartTypeRecommendInputParams = { | ||
export type ChartTypeRecommendInput = { | ||
dataProps: BasicDataPropertyForAdvice[]; | ||
}; | ||
|
||
|
@@ -43,7 +43,11 @@ export type SpecGeneratorInput = { | |
// 单独调用 SpecGenerator 时,还要额外计算 dataProps 么 | ||
dataProps: BasicDataPropertyForAdvice[]; | ||
}; | ||
export type SpecGeneratorOutput = { advices: Advice[] }; | ||
export type SpecGeneratorOutput = { | ||
advices: (Omit<Advice, 'spec'> & { | ||
spec: Record<string, any> | null; | ||
})[]; | ||
}; | ||
|
||
export type VisualEncoderInput = { | ||
chartType: ChartId; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
如果注册多个 plugin 作用于同一个 stage 它的表现会如何,顺序是怎么管理的?
问这个问题是看到下面操作生成 spec 其实也是有一个生成 or 操作顺序的,那么其实可以拆多个 plugin 实现,实现业务自定义 theme 之类的需求
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
之前想的是,一个 component 里的 plugin 都是并行的(输入和输出都被约束为一致的),所以没有声明它的执行顺序。

如果需要所说的有一个顺序执行的,感觉可以后续扩展 componet 的定义,如上面的框图所示, component 有点是执行管理器的概念,可以是用扩展定义其类型和嵌套来实现?