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

支持redis集群模式 #1392

Merged
merged 5 commits into from
Feb 22, 2022
Merged

支持redis集群模式 #1392

merged 5 commits into from
Feb 22, 2022

Conversation

nick2wang
Copy link
Collaborator

@nick2wang nick2wang commented Feb 18, 2022

相关issues:#720 #895 #1238 #1243

redis生产环境上了规模的基本都是使用cluster模式,当前版本只支持standalone

修改:
1.实例配置表新增运行模式字段mode,可选standalone或cluster,只有db_type选择redis时可配置,其他时候隐藏
2.redis引擎引入redis.cluster.RedisCluster作为集群连接器

支持redis集群模式
单元测试更新
单元测试更新
@codecov
Copy link

codecov bot commented Feb 18, 2022

Codecov Report

Merging #1392 (a8c9b77) into master (62fedad) will increase coverage by 0.00%.
The diff coverage is 92.30%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1392   +/-   ##
=======================================
  Coverage   75.24%   75.25%           
=======================================
  Files          82       82           
  Lines       12927    12936    +9     
=======================================
+ Hits         9727     9735    +8     
- Misses       3200     3201    +1     
Impacted Files Coverage Δ
sql/engines/redis.py 77.14% <66.66%> (-0.53%) ⬇️
sql/admin.py 92.70% <100.00%> (+0.05%) ⬆️
sql/engines/__init__.py 71.62% <100.00%> (+0.38%) ⬆️
sql/engines/tests.py 99.83% <100.00%> (ø)
sql/form.py 63.63% <100.00%> (+8.08%) ⬆️
sql/models.py 95.65% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 62fedad...a8c9b77. Read the comment docs.

Copy link
Owner

@hhyo hhyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

很赞,感谢贡献

src/init_sql/v1.8.3.sql Show resolved Hide resolved
@LeoQuote
Copy link
Collaborator

要我看直接加个engine也不错,直接继承了重写一个方法就行

历史数据设置默认值
@nick2wang
Copy link
Collaborator Author

要我看直接加个engine也不错,直接继承了重写一个方法就行

确实额外加个engine更方便些,而且也不用特殊处理前端;主要是考虑到engines冗余,就合并在一起了

@LeoQuote
Copy link
Collaborator

要不加 engine 吧, 这样不用写 type, 也不用改数据结构, 啥也不用, 就是覆盖一个方法

@hhyo
Copy link
Owner

hhyo commented Feb 19, 2022

要不加 engine 吧, 这样不用写 type, 也不用改数据结构, 啥也不用, 就是覆盖一个方法

加engine也还是需要识别数据库类型,前台改的地方不少,不过核心在于非集群模式的redis和集群模式是否有必要单独区分一个类型

@nick2wang
Copy link
Collaborator Author

要不加 engine 吧, 这样不用写 type, 也不用改数据结构, 啥也不用, 就是覆盖一个方法

加engine也还是需要识别数据库类型,前台改的地方不少,不过核心在于非集群模式的redis和集群模式是否有必要单独区分一个类型

还是合并使用一个redis engine吧,配置上来说对用户更友好

@LeoQuote
Copy link
Collaborator

前端尽量友好没问题, 后端我觉得有改进空间, 可能存 json 串就行了, 这次 这个pr LGTM, 可能后续可以再改改, 好几个字段都属于 "连接额外参数",可以收到一个字段里面去,反正不需检索

Copy link
Collaborator

@LeoQuote LeoQuote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nick2wang
Copy link
Collaborator Author

前端尽量友好没问题, 后端我觉得有改进空间, 可能存 json 串就行了, 这次 这个pr LGTM, 可能后续可以再改改, 好几个字段都属于 "连接额外参数",可以收到一个字段里面去,反正不需检索

ok

@hhyo
Copy link
Owner

hhyo commented Feb 21, 2022

前端尽量友好没问题, 后端我觉得有改进空间, 可能存 json 串就行了, 这次 这个pr LGTM, 可能后续可以再改改, 好几个字段都属于 "连接额外参数",可以收到一个字段里面去,反正不需检索

提到连接参数,确实是,oracle以及字符集和数据那些都是,后续优化可以增加一个conn_kwargs,保存连接字典参数,也方便个性化配置,剔除model中的冗余字段

@fancy-lee
Copy link
Contributor

redis集群模式下执测试连接报:module 'redis' has no attribute 'cluster'
archery版本1.8.5

1 similar comment
@fancy-lee
Copy link
Contributor

redis集群模式下执测试连接报:module 'redis' has no attribute 'cluster'
archery版本1.8.5

@barryzhao26
Copy link

redis集群模式下执测试连接报:module 'redis' has no attribute 'cluster' archery版本1.8.5

查看了一下Python Redis库版本,3.5.4,看了文档,需要4.1.0开始才支持
自己下载镜像,升了个级,目前Redis cluster能用了,配置一个节点。

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.

5 participants