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

optimization: 任务历史搜索优化 (closed #1856) #1915

Merged

Conversation

ping15
Copy link
Collaborator

@ping15 ping15 commented Nov 3, 2023

PR

CheckList(PR Assigners)

确认已完成以下操作:

  • 已将分支与当前合入分支同步
  • 已在关联 Issue 中补充「功能自测」部分内容
  • 已更新关联 Issue 的状态 backlog -> todo -> for test -> tested(需通过测试同学测试) -> done
  • 将 PR 添加上 pr/reviewable

CheckList(PR Reviewers)

确认已完成以下操作:

  • 检查关联 Issue 「功能自测」部分内容
  • 必要时打回 pr/reviewable,要求重新修改
  • 及时 Approve

@ping15 ping15 closed this Nov 3, 2023
@ping15 ping15 reopened this Nov 3, 2023
@codecov-commenter
Copy link

codecov-commenter commented Nov 3, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (1e25657) 74.96% compared to head (16f1d03) 74.99%.

Files Patch % Lines
apps/node_man/tools/job.py 93.33% 2 Missing ⚠️
apps/node_man/views/meta.py 50.00% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff               @@
##           v2.4.4-dev    #1915      +/-   ##
==============================================
+ Coverage       74.96%   74.99%   +0.02%     
==============================================
  Files             403      403              
  Lines           27856    27890      +34     
==============================================
+ Hits            20883    20916      +33     
- Misses           6973     6974       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ping15
Copy link
Collaborator Author

ping15 commented Nov 3, 2023

自测

  1. filter_condition测试

不带bk_biz_scope(即用户全部业务,使用admin测试)

image

单业务

image

中间数量业务

image

全业务

image

带时间过滤

image
2. job_list测试

单业务

image

中间业务

image

全业务

image

@ping15 ping15 added the pr/reviewable 可被 review 的 PR label Nov 3, 2023

class FilterConditionSerializer(serializers.Serializer):
category = serializers.CharField(label=_("分类"), required=False, default="")
bk_biz_ids = serializers.ListField(label=_("业务列表"), required=False, default=[])
Copy link
Member

Choose a reason for hiding this comment

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

指定 child

@@ -58,11 +62,13 @@ def fetch_cloud_unique_col_count(self, col):
"""
return Cloud.objects.values_list(col, flat=True).distinct().count()

def fetch_Job_unique_col_count(self):
def fetch_Job_unique_col_count(self, search_business=None):
Copy link
Member

Choose a reason for hiding this comment

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

命名规范:Job -> job

def get_job_queryset_with_biz_scope(cls, all_biz_info, biz_info, biz_permission, search_biz_ids, kwargs):
"""
根据用户所拥有的业务权限进行Job的筛选
:param all_biz_info: 所有的业务id
Copy link
Member

Choose a reason for hiding this comment

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

这里似乎也是 dict?

# 业务权限
if search_biz_ids:
# 字典的 in 比列表性能更高
biz_scope = [bk_biz_id for bk_biz_id in search_biz_ids if bk_biz_id in biz_info]
Copy link
Member

Choose a reason for hiding this comment

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

等价操作:set(biz_permission) & set(search_biz_id)

biz_scope_query_q = Q()
elif need_reverse_query:
biz_scope_query_q = reduce(
operator.and_, [Q(bk_biz_scope__contains=bk_biz_id) for bk_biz_id in biz_scope], Q()
Copy link
Member

Choose a reason for hiding this comment

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

not A and not B and not C = not (A or B or C)

"start_time": "2023-10-01 12:00:00",
"end_time": "2023-10-03 12:00:00",
},
{
Copy link
Member

Choose a reason for hiding this comment

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

似乎没有配置 pre-commit ?

self.assertEqual(result, [
{"name": _("任务ID"), "id": "job_id"},
{"name": _("IP"), "id": "inner_ip_list"},
])
Copy link
Member

Choose a reason for hiding this comment

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

可以再补充一个单元测试,验证取反的场景

@ping15 ping15 force-pushed the _v2.4.x/dev_issue#1856 branch 4 times, most recently from 5e7cf8c to 626e387 Compare November 7, 2023 07:46
"""
# 业务权限
if search_biz_ids:
# 字典的 in 比列表性能更高
Copy link
Member

Choose a reason for hiding this comment

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

这个注释似乎和代码没啥关系

apps/node_man/tools/job.py Show resolved Hide resolved
@ping15 ping15 force-pushed the _v2.4.x/dev_issue#1856 branch 11 times, most recently from 72bf7a8 to db90d74 Compare November 17, 2023 08:14
@@ -73,13 +79,28 @@ def fetch_Job_unique_col_count(self):

for job in job_condition:
# 判断权限
if set(job["bk_biz_scope"]) - {biz["bk_biz_id"] for biz in SEARCH_BUSINESS} == set():
if any(biz_id in {biz["bk_biz_id"] for biz in search_business} for biz_id in set(job["bk_biz_scope"])):
Copy link
Member

Choose a reason for hiding this comment

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

虽然是测试代码,但最好把一些公共变量放到 for 循环外面,避免占用内存

  1. set(job["bk_biz_scope"])
  2. {biz["bk_biz_id"] for biz in search_business}

Copy link
Member

@ZhuoZhuoCrayon ZhuoZhuoCrayon left a comment

Choose a reason for hiding this comment

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

LGTM

@ZhuoZhuoCrayon ZhuoZhuoCrayon merged commit 3ff1180 into TencentBlueKing:v2.4.4-dev Nov 20, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/reviewable 可被 review 的 PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants