-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
新增对doris的支持 #2536
新增对doris的支持 #2536
Conversation
支持doris的查询、上线审核 相关讨论:#2175
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2536 +/- ##
==========================================
- Coverage 76.97% 76.82% -0.15%
==========================================
Files 115 117 +2
Lines 15943 16073 +130
==========================================
+ Hits 12272 12348 +76
- Misses 3671 3725 +54 ☔ View full report in Codecov by Sentry. |
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.
不好意思因为doris 和 mysql 的协议是完全兼容的,能否过一遍你所有的方法,能复用的就删除,无法复用的请在方法注释,或代码评论中说明需要覆盖的原因,这样能最大程度的简化代码,减轻审核的难度。
另外CI的这个检查,lint 和覆盖率是必须要过的,可以参考下 black 的使用方法,单元测试这个如果你能力有限,我有空帮你补一下也是可以的
class DorisEngine中的改动说明 auto_backup: server_version: query: get_all_databases: query_check: execute_check: execute_workflow: execute: |
单元测试实在是不会,请帮忙添加 |
https://github.com/Grain-Yu/Archery/pull/1 把这个合了看一下, 下次提 pr 记得新开一个分支, 主分支有保护我这边不能直接推 |
reuse get_all_databases
@hhyo 我帮忙写了点测试覆盖率还是低了点, 不过没关系这是 engine 级别的, 不会危害到主程序的监控, 等有空我再归并一些重复的优化一下 |
@Grain-Yu 另外我这边没 doris 的环境, 还请帮忙测试下能否正常运行 |
支持doris的查询、上线审核
相关讨论:#2175