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

feature: 提供 Agent 包管理后台接口 (closed #1683) #1844

Closed

Conversation

wyyalt
Copy link
Collaborator

@wyyalt wyyalt commented Oct 9, 2023

PR

CheckList(PR Assigners)

确认已完成以下操作:

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

CheckList(PR Reviewers)

确认已完成以下操作:

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

@codecov-commenter
Copy link

codecov-commenter commented Oct 9, 2023

Codecov Report

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

Comparison is base (2a7ec66) 74.71% compared to head (1b0d11c) 74.70%.
Report is 1 commits behind head on v2.4.4-dev.

Files Patch % Lines
common/utils/drf_utils.py 53.74% 68 Missing ⚠️
apps/node_man/views/package_manage.py 64.40% 21 Missing ⚠️
...ackend/subscription/steps/agent_adapter/adapter.py 84.31% 8 Missing ⚠️
apps/node_man/handlers/meta.py 25.00% 6 Missing ⚠️
apps/node_man/serializers/package_manage.py 93.50% 5 Missing ⚠️
apps/node_man/permissions/package_manage.py 72.72% 3 Missing ⚠️
apps/node_man/serializers/job.py 66.66% 2 Missing ⚠️
apps/node_man/utils/filters.py 85.71% 2 Missing ⚠️
apps/backend/subscription/views.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff               @@
##           v2.4.4-dev    #1844      +/-   ##
==============================================
- Coverage       74.71%   74.70%   -0.01%     
==============================================
  Files             404      411       +7     
  Lines           28246    28698     +452     
==============================================
+ Hits            21103    21439     +336     
- Misses           7143     7259     +116     

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

@wyyalt wyyalt force-pushed the _v2.4.x/dev_issue#1683 branch 2 times, most recently from 781da34 to 26bc8ca Compare October 13, 2023 08:46
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.

整体上有几个问题:

  1. 请求参数不完善,没有相关的字段描述
  2. 接口注释不完善,不知道哪个接口用在哪个页面,对应关系要补充
  3. swagger_auto_schema 引入目的是?尽可能整个项目用一套

apps/node_man/models.py Show resolved Hide resolved

filter_fields = ("module", "creator", "is_ready", "version")

@swagger_auto_schema(
Copy link
Member

Choose a reason for hiding this comment

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

这里为什么不用原生的 swagger_auto_schema

]

mock_os_cpu_arch = [
{"name": "Linux_x86_64", "id": "linux_x86_64", "host_count": 10},
Copy link
Member

Choose a reason for hiding this comment

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

看设计稿,应该是指维度聚合后的插件包的数量?

我感觉这个地方是不是耦合了两个功能,获取快捷筛选可选项,以及获取版本包对应的「已部署主机」数量?

这里需要拆成两个接口,统计「已部署主机」是一个耗时接口,需要单独提供一个接口

image

image

operation_summary="获取快速筛选信息",
tags=PACKAGE_MANAGE_VIEW_TAGS,
)
@action(detail=False, methods=["GET"])
Copy link
Member

Choose a reason for hiding this comment

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

缺少参数

  1. agent or proxy

"packages": [
{
"pkg_abs_path": "xxx/xxxxx",
"pkg_name": "gseagent_2.1.7_linux_x86_64.tgz",
Copy link
Member

Choose a reason for hiding this comment

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

设计稿的包名称有点问题,这里的 mock 数据按我们真实的来:gse_agent-v2.1.3-beta.18.tgz

from apps.node_man.models import AgentPackages


class AgentPackageSerializer(serializers.ModelSerializer):
Copy link
Member

Choose a reason for hiding this comment

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

序列化器的字段描述要再完善下, review 理解起来有点难

"tags": [{"id": "stable", "name": "稳定版本"}],
"creator": "string",
"pkg_ctime": "2019-08-24 14:15:22",
"host_count": 100,
Copy link
Member

Choose a reason for hiding this comment

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

host_count 属于耗时查询,需要和列表拉取逻辑解耦,单独设计出按 pkg_ids 拉取部署数量的接口

@swagger_auto_schema(
operation_summary="操作类动作:启用/停用",
body_in=pkg_manage.OperateSerializer,
responses={200: pkg_manage.SearchResponseSerializer},
Copy link
Member

Choose a reason for hiding this comment

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

为什么 update 是返回 SearchResponseSerializer

# ordering_fields = ("module",)
serializer_class = pkg_manage.PackageSerializer
filter_backends = (DjangoFilterBackend, filters.SearchFilter, filters.OrderingFilter)

Copy link
Member

Choose a reason for hiding this comment

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

缺 PermissionClass,这个 PermissionClass 可以限制目前仅限超管访问


class SearchSerializer(serializers.Serializer):
os_cpu_arch = serializers.CharField(required=False)
tags = serializers.ListField(required=False)
Copy link
Member

Choose a reason for hiding this comment

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

复用 filterset_fields 实现基础的筛选,参考:

  1. GSEKit:https://github.com/TencentBlueKing/bk-process-config-manager/blob/V1.0.X/apps/gsekit/job/views.py
  2. 扩展 DjangoFilterBackend,实现额外字段(例如 tag)的筛选。

@wyyalt wyyalt changed the base branch from v2.4.2-dev to v2.4.4-dev November 8, 2023 08:08
@wyyalt wyyalt force-pushed the _v2.4.x/dev_issue#1683 branch from 26bc8ca to 6aa02ca Compare November 8, 2023 08:11
@wyyalt wyyalt force-pushed the _v2.4.x/dev_issue#1683 branch 7 times, most recently from 6964d08 to 9b1ea46 Compare November 29, 2023 03:27
@wyyalt wyyalt force-pushed the _v2.4.x/dev_issue#1683 branch 3 times, most recently from 2ca9717 to 1b0d11c Compare January 5, 2024 09:31
@wyyalt wyyalt force-pushed the _v2.4.x/dev_issue#1683 branch from 1b0d11c to 038f301 Compare January 9, 2024 08:28
@wyyalt wyyalt force-pushed the _v2.4.x/dev_issue#1683 branch from 038f301 to 9529982 Compare January 9, 2024 08:39
@wyyalt wyyalt closed this Jan 9, 2024
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