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

[fix] 修复延迟数据存放位置不对的问题 #860

Merged
merged 2 commits into from Dec 22, 2023
Merged

[fix] 修复延迟数据存放位置不对的问题 #860

merged 2 commits into from Dec 22, 2023

Conversation

ghost
Copy link

@ghost ghost commented Dec 4, 2023

  • 我学习源代码的时候发现 判断延迟数据存放位置 的逻辑不是很对。翻看 PR记录 里说 history 主要存 ProxyProvider 的健康检查的延迟数据,extra 存 ProxyGroup 对应的延迟数据。按照现有逻辑,应该用测试的 url 与 ProxyProvider 里 healthcheck 的 url 进行比较(如果 Proxy 来自于某个 ProxyProvider 的话),如果 url 一致 或者 没有 healthcheck 则存 history,否则存 extra。但是,Proxy 并没有持有 healthcheck 的 url,导致存的位置不一定对。

例外我有个疑惑,为什么不直接把所有的数据都存在 extra 这样的 Map 里呢?是因为 涉及到 Dashboard 解析数据吗?个人感觉我这个 PR 的方案不够优雅😳,最好的办法还是把两个数据合到一起,这样就不用判断存哪了

@ghost
Copy link
Author

ghost commented Dec 5, 2023

重新实现了一下,把数据都放到 histories 里了。

可能需要前端修改一下解析数据的方式

@kunish
Copy link

kunish commented Dec 5, 2023

重新实现了一下,把数据都放到 histories 里了。

可能需要前端修改一下解析数据的方式

前端可以改,还是希望 API 能更清晰一些,现在前端的实现很迷

@Larvan2
Copy link
Collaborator

Larvan2 commented Dec 5, 2023

@wzdnzd Any comments?

@ghost
Copy link
Author

ghost commented Dec 5, 2023

重新实现了一下,把数据都放到 histories 里了。

可能需要前端修改一下解析数据的方式

前端可以改,还是希望 API 能更清晰一些,现在前端的实现很迷

延迟相关的数据,由原来的 historyextra 两个字段合并为 histories 一个字段了,拿着测试 URL 作为 key 去 histories 里取数据,histories 的 JSON 数据格式如下:

延迟相关的数据全部放到 extra 里了(原来的 history 字段仍保留),拿着测试 URL 作为 key 去 extra 里取数据,extra 的 JSON 数据格式如下

{
    "url1": {
        "alive": true,
        "history": [
            {
                "time": "2023-12-05T19:06:02.636288+08:00",
                "delay": 292
            },
            {
                "time": "2023-12-05T19:06:02.7369547+08:00",
                "delay": 376
            }
        ]
    },
    "url2": {
        "alive": false,
        "history": [
            {
                "time": "2023-12-05T19:06:02.636288+08:00",
                "delay": 231
            },
            {
                "time": "2023-12-05T19:06:02.7369547+08:00",
                "delay": 0
            }
        ]
    }
}

@snakem982
Copy link

感觉这改动有点儿大啊,希望添加新功能的同时可以向下兼容

@wzdnzd
Copy link

wzdnzd commented Dec 5, 2023

@wzdnzd Any comments?

如果前端愿意适配的话,合并了也挺好,统一。之前之所以放到 extra 里,主要就是怕影响到 dashboard,还有就是尽量保持兼容

@wzdnzd
Copy link

wzdnzd commented Dec 5, 2023

重新实现了一下,把数据都放到 histories 里了。

可能需要前端修改一下解析数据的方式

前端可以改,还是希望 API 能更清晰一些,现在前端的实现很迷

@kunish 顺便问一下,支持不同代理组配置不同测试 URL 这个考虑吗?

@kunish
Copy link

kunish commented Dec 5, 2023

重新实现了一下,把数据都放到 histories 里了。

可能需要前端修改一下解析数据的方式

前端可以改,还是希望 API 能更清晰一些,现在前端的实现很迷

@kunish 顺便问一下,支持不同代理组配置不同测试 URL 这个考虑吗?

可以考虑

@wzdnzd
Copy link

wzdnzd commented Dec 5, 2023

重新实现了一下,把数据都放到 histories 里了。

可能需要前端修改一下解析数据的方式

前端可以改,还是希望 API 能更清晰一些,现在前端的实现很迷

@kunish 顺便问一下,支持不同代理组配置不同测试 URL 这个考虑吗?

可以考虑

期待ing,感谢

@wwqgtxx
Copy link
Collaborator

wwqgtxx commented Dec 5, 2023

不希望有任何修改原clash本来有的restful api的改动出现,可以有其他附加属性,但请不要修改或删除原有的属性和字段,这种大规模破坏兼容性的修改将不会被合并

@ghost
Copy link
Author

ghost commented Dec 6, 2023

不希望有任何修改原clash本来有的restful api的改动出现,可以有其他附加属性,但请不要修改或删除原有的属性和字段,这种大规模破坏兼容性的修改将不会被合并

好的,我已经将代码回滚到一开始的方案了,但是看上去在 provider.go 里调用一下方法看上去有些奇怪,你有什么好的建议吗?

@wwqgtxx 现在改成在延迟数据不管是 ProxyGroup 配置的还是 ProxyProvider 里配置的都在 extra 里存一份,对原有的接口不造成破坏,你看可以不?或者有什么建议吗?

@ghost
Copy link
Author

ghost commented Dec 20, 2023

@Larvan2 @wwqgtxx 这个PR还合吗?或者给个意见我再改改也行😳

@wwqgtxx
Copy link
Collaborator

wwqgtxx commented Dec 20, 2023

@tommytag 我倒是不反对

@ghost
Copy link
Author

ghost commented Dec 20, 2023

@tommytag 我倒是不反对

感谢回复,如果不反对的话我申请合并进Alpha

@Larvan2 Larvan2 merged commit fe9af3c into MetaCubeX:Alpha Dec 22, 2023
Larvan2 added a commit that referenced this pull request Dec 22, 2023
* [fix] incorrect data save location for latency

* healthcheck latency of the provider is also stored in the extra, with…
Larvan2 added a commit that referenced this pull request Dec 22, 2023
* [fix] incorrect data save location for latency

* healthcheck latency of the provider is also stored in the extra, without compromising rest api compatibility

Co-authored-by: tommytag <tommytag@outlook.com>
Larvan2 added a commit that referenced this pull request Dec 22, 2023
@ghost ghost mentioned this pull request Jan 5, 2024
@qwerr0
Copy link

qwerr0 commented Jan 15, 2024

一个月过去了, 请问一下还没有 pull 进去吗?

@PuerNya
Copy link

PuerNya commented Jan 15, 2024

@qwerr0 为什么要回复一个 merged 的 pr

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.

8 participants