-
Notifications
You must be signed in to change notification settings - Fork 470
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: add api-workflow plugin #1229
base: main
Are you sure you want to change the base?
Conversation
@lixf311 上面的CLA要签署一下 |
ok~ |
@lixf311 加一下我钉钉吧:chengtanzty |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1229 +/- ##
==========================================
+ Coverage 35.91% 35.96% +0.05%
==========================================
Files 69 69
Lines 11576 9500 -2076
==========================================
- Hits 4157 3417 -740
+ Misses 7104 5767 -1337
- Partials 315 316 +1 |
@@ -0,0 +1,376 @@ | |||
## 功能说明 |
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.
README的格式可以参考下ai-proxy的方便后续放到官网上
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.
已修改
| service_headers | array of header object | 选填 | | 请求的头 | | | ||
| service_body_replace_keys| array of bodyReplaceKeyPair object | 选填| 请求body模板替换键值对 | 用来构造请求| 如果为空,则直接使用service_body_tmpl请求 | | ||
| service_body_tmpl | string | 选填 | | 请求的body模板 | | | ||
| service_type | string | 必填 | | 请求的类型 | static,domain | |
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.
这里不需要区分service_type,Cluster使用FQDNCluster可以适配所有方式。配置时,如果是static类型,service_name使用xxx.static,如果是dns类型,service_name使用xxx.dns
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.
原来是这样,是我理解的不对,感谢指点,我来改下
func onHttpRequestBody(ctx wrapper.HttpContext, config PluginConfig, body []byte, log wrapper.Log) types.Action { | ||
|
||
initHeader := make([][2]string, 0) | ||
//初始化运行状态 |
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.
注释格式可以调整下 统一成//后面加个空格
// 初始化运行状态
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.
已修改
|
||
initHeader := make([][2]string, 0) | ||
//初始化运行状态 | ||
workflowExecStatus, err := initWorkflowExecStatus(config) |
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.
这个初始状态是否可以在parseConfig的时候就定义好,我看这里只用到了config的信息,不涉及每个请求的具体信息
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.
已放到了parseConfig里
initHeader := make([][2]string, 0) | ||
//初始化运行状态 | ||
workflowExecStatus, err := initWorkflowExecStatus(config) | ||
log.Errorf("init status : %v", workflowExecStatus) |
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.
日志级别调整下
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.
已修改
} | ||
return | ||
|
||
}, uint32(maxDepth)*5000) |
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.
这个超时时间最好改为可配置的
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.
这部分已经调整了,在config里加了一个timeout的配置项
*/ | ||
type CompareFunc func(a, b float64) bool | ||
|
||
var operators = map[string]interface{}{ |
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.
由于tinygo的一些限制(比如不能使用反射等)不能直接使用类似https://github.com/vulcand/predicate 的go第三方库,我们可以参考下这个库的设计和实现,丰富一些类似And和Or的能力(只是建议,可以优化下代码实现,具体是否丰富功能看当前场景下是否需要)
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.
我想了想,加上了and or这种能力,别的还在考虑ing
w.Task.Cluster = wrapper.FQDNCluster{ | ||
FQDN: node.ServiceName, | ||
Port: node.ServicePort, | ||
} |
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.
前面README中提到的只保留这个即可
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.
已修改
@@ -0,0 +1,21 @@ | |||
module github.com/alibaba/higress/plugins/wasm-go/extensions/ai-workflow |
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.
module名称叫ai-workflow即可
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.
已修改
"github.com/higress-group/proxy-wasm-go-sdk/proxywasm/types" | ||
"github.com/tidwall/gjson" | ||
"net/http" | ||
) |
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.
调整下go import的顺序吧,代码编辑器一般都有这个设置
goland的话可以参考下这个:https://blog.csdn.net/t949500898/article/details/123330231
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.
学习了,已经设置,感谢。
|
||
var operators = map[string]interface{}{ | ||
//CompareFunc 用来比较float64 数据大小: | ||
"eq": func(a, b float64) bool { return a == b }, |
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.
只支持数值类型的比较吗?这种场景下字符串的比较可能更常见一些
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.
可以加,应该没啥问题,我来想想增加哪些
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.
已经添加了字符串的比较
Ⅰ. Describe what this PR did
Add ai-workflow plugin,the first version only supports tools.
Ⅱ. Does this pull request fix one issue?
fixes #1225
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews