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

mysql会话管理相关函数迁移到engine #1661

Merged
merged 1 commit into from
Jul 9, 2022
Merged

Conversation

weideguo
Copy link
Collaborator

@weideguo weideguo commented Jul 7, 2022

进行一些函数迁移以及增加一些相关测试,没有涉及功能改动。
关联之前的pr #1563

@codecov
Copy link

codecov bot commented Jul 7, 2022

Codecov Report

Merging #1661 (6762259) into master (2ffb6de) will increase coverage by 0.53%.
The diff coverage is 77.35%.

@@            Coverage Diff             @@
##           master    #1661      +/-   ##
==========================================
+ Coverage   75.71%   76.24%   +0.53%     
==========================================
  Files          91       91              
  Lines       14212    14298      +86     
==========================================
+ Hits        10760    10901     +141     
+ Misses       3452     3397      -55     
Impacted Files Coverage Δ
sql/db_diagnostic.py 17.39% <0.00%> (+2.57%) ⬆️
sql/engines/mongo.py 50.00% <46.15%> (+5.08%) ⬆️
sql/engines/mysql.py 82.78% <97.29%> (+2.40%) ⬆️
sql/engines/tests.py 99.77% <98.78%> (-0.07%) ⬇️

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 2ffb6de...6762259. 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.

rds的判断也放到mysql engine内是不是可以,调用完全忽略db类型判断

@weideguo
Copy link
Collaborator Author

weideguo commented Jul 8, 2022

但这样会照成engine耦合太高

@LeoQuote
Copy link
Collaborator

LeoQuote commented Jul 8, 2022

同意 @weideguo 的说法, 目前的版本应该不错的

@weideguo
Copy link
Collaborator Author

weideguo commented Jul 8, 2022

有个想法
sql/engines/cloud/aliyun_rds.py 创建AliyunRDS

from sql.engines.mysql import MysqlEngine

class AliyunRDS(MysqlEngine):
    
    # 将sql/aliyun_rds.py的函数迁移值此
    def processlist(self, instance_name, command_type):
        pass
    

sql/engines/__init__.py 修改get_engine函数

def get_engine(instance=None): 
    """获取数据库操作engine"""
    if instance.db_type == "mysql":
        if AliyunRdsConfig.objects.filter(instance=instance, is_enable=True).exists():
            from .cloud.aliyun_rds import AliyunRDS
            return AliyunRDS(instance=instance)
        else:
            from .mysql import MysqlEngine
            return MysqlEngine(instance=instance)

但我没有阿里云的环境,不敢直接拆

@hhyo
Copy link
Owner

hhyo commented Jul 8, 2022

俺也没有环境了,不过这样改挺好,可以单独开pr,这个就这样子就行

@LeoQuote
Copy link
Collaborator

LeoQuote commented Jul 8, 2022

可以另提pr等人验证,😄

@hhyo hhyo merged commit a8290d7 into hhyo:master Jul 9, 2022
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.

3 participants