-
Notifications
You must be signed in to change notification settings - Fork 44
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
add engine params to engine API #641
Conversation
utils/cache.go
Outdated
@@ -39,3 +39,12 @@ func (c *EngineCache) Get(endpoint string) engine.API { | |||
func (c *EngineCache) Delete(host string, _ ...string) { | |||
c.cache.Delete(host) | |||
} | |||
|
|||
func (c *EngineCache) Items() map[string]engine.API { |
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.
唉为啥要强转回 Engine 实体
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-cache包一层就是为了不用直接搞any,不过确实可以拿掉,少一次遍历,我其实想把这里的cache换成haxmap,cache主要的用途就是那个自动过期清理,但是按现在的实现代码已经明确清理了无效的engine,cache这个功能没啥必要了,而且ForEach遍历的时候就只遍历一次,go-cache用items来每一次都要遍历2次
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.
我觉得你第二个思路可以……主要是吧,不想把 engine 内部细节暴露到外层来
UT主要还是是那个并发的问题 |
engine/factory/factory.go
Outdated
} | ||
|
||
// Get . | ||
func (e *EngineCache) Get(key string) engine.API { | ||
return e.cache.Get(key) | ||
if api, found := e.cache.Get(key); found { |
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.
我记得默认行为没 key api 就为 nil 吧
LGTM |
enginefactory has two map, cache is cache key to engine API and keysToCheck is cache key to engineParams. this pr mainly does 2 things: