fix: switch Sina and Tencent endpoints to HTTPS - #1215
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9fc3e4f76
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
🤖 自动审查报告
📁 修改的文件
🧠 AI 代码审查意见结论: Ready to Merge 审查结果
|
f9fc3e4 to
39b453d
Compare
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性:通过,将新浪和腾讯实时行情接口切换到 HTTPS 有明确安全收益。
- 是否有对应 issue:有,Refs #1157。
- PR 类型:fix,核心是修复数据源实时行情请求协议,附带更新
docs/CHANGELOG.md。 - description 完整性:不完整,缺少 provider HTTPS 支持的官方来源或等价长期稳定性证据;验证描述称 parser 已验证,但给出的可复现命令主要是
curl -I,没有展示实际解析路径的运行结果。 - 是否可直接合入:不可,需补齐外部接口兼容性证据和实际解析 smoke 结果后再合入。
主要问题
- [Process blocker]
data_provider/akshare_fetcher.py将hq.sinajs.cn和qt.gtimg.cn的 Base URL 从 HTTP 改为 HTTPS,属于外部 provider endpoint 语义变更。PR 描述只说明 curl 证书和 200 OK,未提供官方支持来源或等价稳定性依据,也未展示当前运行时对 GBK 响应内容完成解析的命令与输出。风险是 HTTPS endpoint 虽能连通,但响应、编码、Referer 校验或长期兼容行为与 HTTP 不完全一致时,会影响实时行情 fallback 链路。建议补充官方/长期兼容依据,并贴出调用本仓库 Sina/Tencent 解析路径的 smoke 结果。 - [Process blocker] PR 描述中的兼容性结论写为 “None”,但本次实际修改外部行情接口协议和 Referer。按照 AGENTS.md 对数据源 fallback 与外部依赖变更的稳定性要求,应明确说明旧 HTTP 路径的回退方式、失败降级是否仍按现有 circuit breaker/fallback 工作,以及是否覆盖了新浪、腾讯两条实时行情路径。
🤖 此回复由 OpenReview Bot 自动生成,仅供参考。如有疑问请 @维护者。
There was a problem hiding this comment.
整体方向和当前 diff 都是合理的:本 PR 已经把 #1157 中的 HTTPS endpoint 变更拆成了足够小的 focused PR,代码层面只修改 Sina/Tencent realtime URL 与 Referer,并补了 changelog。我本地用 PR 代码做过实际解析 smoke,_get_stock_realtime_quote_sina("600519") 和 _get_stock_realtime_quote_tencent("600519") 都能通过 HTTPS 返回 UnifiedRealtimeQuote,所以目前没有看到代码正确性阻断问题。
合入前仍建议补齐:
PR description 里的验证证据需要从 curl -I 升级为实际 parser smoke 结果。curl -I 只能证明连通/证书,不能证明 GBK payload 和字段解析路径可用;而且 Sina 在不带 Referer/User-Agent 时可能返回 403。建议贴出调用本仓库 Sina/Tencent 解析函数的命令和关键输出。
非阻断建议:可以在 tests/test_akshare_realtime_logging.py 里顺手让 mocked requests.get 断言 URL 使用 https://、Referer 使用 HTTPS。这个 PR 的核心行为就是 URL/Referer 变更,加一个很小的断言能防止后续回退。
ZhuLinsen
left a comment
There was a problem hiding this comment.
跟进结论
- 结论:部分接受;当前 diff 确实已同时覆盖 Sina/Tencent 的 HTTPS URL 与 Referer,且补充的实际解析 smoke 能收缩此前“解析路径未验证”的风险判断。
- 是否仍有阻断:无;CI 已通过,当前代码改动范围很小,未看到会阻止合入的代码正确性问题,
mergeable_state=blocked不单独构成阻断。
说明
data_provider/akshare_fetcher.py:Sina 与 Tencent 两条 realtime 路径均已从http://改为https://,对应 Referer 也同步改为 HTTPS;早先“只改了 Tencent、Sina 仍为 HTTP”的判断已不适用于当前 head。data_provider/akshare_fetcher.py:讨论中已补充_get_stock_realtime_quote_sina("600519")和_get_stock_realtime_quote_tencent("600519")的实际解析 smoke 结论,足以降低仅靠curl -I不能证明解析兼容的担忧。docs/CHANGELOG.md:Unreleased 条目为扁平格式,且描述与本次用户可见变更一致。- 剩余建议:PR 描述里的验证命令仍主要是
curl -I,建议把实际解析 smoke 命令/结果补进去,便于后续追溯;但在当前已有讨论证据和 CI 通过的情况下,我不再把这点作为合入阻断。
🤖 此回复由 OpenReview Bot 自动生成,仅供参考。如有疑问请 @维护者。
|
@ZhuLinsen updated the PR description with the parser execution results as suggested. |
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性:通过,将 Sina/Tencent 实时行情请求从 HTTP 切到 HTTPS 有明确安全收益,且符合最小修复范围。
- 是否有对应 issue:有,Refs #1157。
- PR 类型:fix + docs,核心是修复数据源请求协议,附带更新
docs/CHANGELOG.md。 - description 完整性:不完整,已补充 smoke 与回滚说明,但仍缺少 provider HTTPS 支持的官方来源或等价长期稳定性证据。
- 是否可直接合入:不可,当前 CI 虽已通过且代码 diff 很小,但本 PR 修改外部实时行情 Base URL,合入前应补齐外部接口兼容性证据。
主要问题
- [Process blocker]
data_provider/akshare_fetcher.py:本 PR 将hq.sinajs.cn、qt.gtimg.cn的运行时请求协议从 HTTP 改为 HTTPS,属于外部 provider endpoint/Base URL 语义变更。PR 描述已给出curl -I和解析 smoke 结果,也有 revert 回滚方案,但没有给出 Sina/Tencent HTTPS 长期支持的官方来源链接或等价稳定性依据。风险是第三方接口在不同网络、Referer、证书链或后续策略变化下出现兼容性问题时,仓库缺少可追溯依据。建议在 PR 描述中补充官方来源或维护者认可的稳定性证据;若无法提供,至少明确这是基于当前 live smoke 的兼容性判断,并保留 HTTP 回退路径/验证说明。
🤖 此回复由 OpenReview Bot 自动生成,仅供参考。如有疑问请 @维护者。
39b453d to
188a96b
Compare
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性:通过,将 Sina/Tencent 实时行情请求从 HTTP 切换到 HTTPS 有明确安全收益,且改动范围聚焦在数据源请求协议。
- 是否有对应 issue:有,Refs #1157。
- PR 类型:fix/docs + 修复实时行情外部接口协议,附带更新
docs/CHANGELOG.md与相关测试。 - description 完整性:不完整 + 已补充 smoke、解析结果、回滚方案和 changelog,但仍缺少 provider HTTPS 支持的官方来源或等价长期稳定性证据。
- 是否可直接合入:不可 + 当前 CI 已通过,代码 diff 本身未看到明显正确性问题,但本 PR 修改外部实时行情 Base URL,按仓库稳定性护栏需要补齐外部接口兼容性证据后再合入。
主要问题
data_provider/akshare_fetcher.py:本 PR 将 Sina/Tencent 实时行情请求与 Referer 从 HTTP 切到 HTTPS,属于外部数据源 Base URL 变更。PR 描述已给出curl -I和 parser smoke 结果,能说明当前运行时可用,但还没有 provider 官方来源或等价长期稳定性证据来证明 HTTPS endpoint 是受支持入口。风险是上游 HTTPS 行为、证书、重定向或响应编码策略变化时,两个 fallback realtime 来源可能同时受影响;建议补充官方文档/页面引用,或说明无法取得官方来源并提供可接受的长期稳定性证据。
🤖 此回复由 OpenReview Bot 自动生成,仅供参考。如有疑问请 @维护者。
188a96b to
22ddc99
Compare
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性:通过,将 Sina/Tencent 实时行情请求从 HTTP 切换到 HTTPS 有明确安全收益,改动范围也聚焦。
- 是否有对应 issue:有,Refs #1157。
- PR 类型:fix + docs,核心是修复外部实时行情接口协议,附带更新
docs/CHANGELOG.md和相关测试断言。 - description 完整性:不完整,已包含动机、范围、smoke、解析示例、风险评估和回滚方案,但仍缺少 provider HTTPS 支持的官方来源或等价长期稳定性证据。
- 是否可直接合入:不可,当前 CI 已通过且本地 diff 未见代码正确性问题,但本 PR 修改外部行情接口 Base URL,仍需补齐兼容性证据后再合入。
主要问题
- [Process blocker]
data_provider/akshare_fetcher.py:本 PR 将hq.sinajs.cn与qt.gtimg.cn的实时行情 Base URL 从 HTTP 改为 HTTPS,并同步修改 Referer。PR 描述提供了curl -I和解析 smoke 示例,测试也补了 HTTPS URL 断言,但没有提供 Sina/Tencent 对这些 HTTPS endpoint 的官方来源、稳定性说明,或等价的长期可用性证据。根据仓库规范,外部 provider/Base URL 变更需要评估运行时兼容性、回退路径和风险;当前缺口会导致未来 provider TLS、Referer、编码或反爬策略变化时,两个备选实时源可能同时失效且难以判断是否属于预期迁移风险。建议在 PR 描述中补充官方链接或维护者认可的长期稳定性依据;如果没有官方文档,也应明确说明证据来源、验证日期、影响范围和回退方式。
🤖 此回复由 OpenReview Bot 自动生成,仅供参考。如有疑问请 @维护者。
|
I’ve looked into the stability concerns for the Sina/Tencent endpoints. Since qt.gtimg.cn (Tencent) is known to have spotty HTTPS support, I’d like to propose a more robust 'HTTPS-First with HTTP Fallback' implementation. Instead of a hard switch, I can refactor akshare_fetcher.py to use a helper that attempts the HTTPS connection first but automatically falls back to HTTP if a connection or SSL error occurs. This ensures we get the security benefits where possible without risking data downtime. I can have this update ready quickly if you're open to reopening the PR. What do you think? |
PR Type
Background And Problem
Sina and Tencent realtime stock endpoints are currently accessed via HTTP, which transmits financial data in plaintext. Switching to HTTPS improves security and is supported by the providers.
Scope Of Change
data_provider/akshare_fetcher.pyto usehttps://forhq.sinajs.cnandqt.gtimg.cn.Refererheaders to use HTTPS for consistency.docs/CHANGELOG.md.Issue Link
Refs #1157 (Follow-up focused PR as requested by maintainer).
Verification Commands And Results
Verified that both the network connectivity and the internal parsers work correctly with HTTPS.
1. Network Smoke Test (HTTPS Connectivity)
2. Parser Integration Test (Live Execution)
Verified the end-to-end flow from fetching data via HTTPS to parsing it into
UnifiedRealtimeQuoteobjects.Sina Parser Output Example:
Tencent Parser Output Example:
Compatibility And Risk
None. Verified that field mapping, GBK/UTF-8 encoding, and response formats remain identical between HTTP and HTTPS.
Rollback Plan
Revert this PR to return to HTTP-based endpoints.
Checklist
docs/CHANGELOG.md/ Relevant docs anddocs/CHANGELOG.mdare updated