Dev/account multi refactor #426

huerni · 2025-01-24T03:05:56Z

No description provided.

protos/PublicDefs.proto

src/CraneCtld/AccountManager.h

RileyWen · 2025-02-13T06:02:41Z

src/CraneCtld/AccountManager.cpp

+  if (!user_result) return std::unexpected(user_result.error());
+  const User* op_user = user_result.value();
+  result = CheckIfUserHasPermOnUserOfAccountNoLock_(*op_user, p,
+                                                    &actual_account, false);


这个地方还是语义太松了啥东西都往参数里塞后面这东西没法维护

这个不太好改，参数都是必需的，除非把这个函数拆了

src/CraneCtld/CtldGrpcServer.cpp

protos/Crane.proto

L-Xiafeng · 2025-02-25T07:49:20Z

src/CraneCtld/CtldGrpcServer.cpp

+          request->uid(), request->name(), qos_list, request->force());
+      if (!rich_res)
+        response->mutable_rich_error_list()->Add()->CopyFrom(rich_res.error());
+    }


这里后面加一个else std::unreachable

不是这地方参数有点套娃了吧直接统一unordered set就好了不需要list和set转来转去

partition 可以用unordered set，SetAccountAllowedQos 需要保证顺序，这里是和SetAccountAllowedQos 保持统一了

也不是保证顺序，因为SetAccountAllowedQos需要设置default_qos 都换成unordered set也可以，default_qos可以在request中摘出来，然后单独传进去

这里defaultqos也不支持批量吧？default和普通的走不一样的逻辑就好了。
把defaultqos直接作为user的属性，然后可以普通的换成unorderedset

原逻辑是 SetAccountAllowedQos时，如果default_qos需要更换，是把传入的第一个作为default qos，我这里遵循了原逻辑为了保证顺序所以这么写的整体换成unorderedset也可以，只要在换成unorderedset之前确定default_qos，就能和原逻辑打平了

这里还是改一下吧，这样依赖顺序感觉有点怪？我看其他的default情况都不是这样的

这里改成两个参数，一个std optional default_qos,一个set是普通的qos

src/CraneCtld/AccountManager.cpp

src/CraneCtld/CtldGrpcServer.cpp

src/CraneCtld/AccountManager.cpp

RileyWen · 2025-03-08T14:16:13Z

protos/Crane.proto

@@ -352,7 +352,7 @@ message QueryAccountInfoReply {

 message QueryUserInfoRequest {
  uint32 uid = 1;
-  string name = 2;
+  repeated string user_list = 2;
  string account = 3;


为什么上面是list 但是account不是list

这个account是default account吗

RileyWen · 2025-03-08T14:16:26Z

protos/Crane.proto

@@ -377,13 +377,13 @@ message BlockAccountOrUserRequest {
  uint32 uid = 1;
  bool block = 2;
  EntityType entity_type = 3;
-  string name = 4;
+  repeated string entity_list = 4;
  string account = 5;


同上的问题

RileyWen · 2025-03-08T14:16:48Z

protos/PublicDefs.proto

@@ -335,6 +335,11 @@ message TrimmedPartitionInfo {
  repeated TrimmedCranedInfo craned_lists = 3;
 }

+message RichError {
+  string description = 1;


Code在前 description在后

RileyWen · 2025-03-08T14:25:26Z

src/CraneCtld/CtldGrpcServer.cpp

-  auto res = g_account_manager->DeleteAccount(request->uid(), request->name());
-  if (res) {
+  for (const auto &account_name : request->account_list()) {
+    auto res = g_account_manager->DeleteAccount(request->uid(), account_name);


为啥有的地方是里面for循环有的地方是外面for循环

RileyWen · 2025-03-08T14:42:59Z

src/CraneCtld/AccountManager.cpp

  util::write_lock_guard user_guard(m_rw_user_mutex_);
  util::read_lock_guard account_guard(m_rw_account_mutex_);

-  const User* p = GetExistedUserInfoNoLock_(name);
+  const User* p = GetExistedUserInfoNoLock_(username);


这种地方不要获取了指针但是又是在另外一个函数的里面检查这个指针是不是空很容易漏掉要么就直接封一个CheckIfUidHasPermOnUserOfAccountNoLock_

RileyWen · 2025-03-08T15:19:19Z

src/CraneCtld/CtldGrpcServer.cpp

  }

-  for (const auto &it : res_user_map) {
+  for (const auto &it : res.value()) {
    const auto &user = it.second;
    for (const auto &[account, item] : user.account_to_attrs_map) {
      if (!request->account().empty() && account != request->account()) {


这地方逻辑写的好奇怪下面account就是一个ranges find就可以解决吧然后其他user的信息设置提到外面去就行吧

huerni marked this pull request as ready for review February 8, 2025 07:44

huerni marked this pull request as draft February 8, 2025 08:25

huerni force-pushed the dev/account_multi_refactor branch from d0ba76c to e61cd33 Compare February 10, 2025 06:38

huerni marked this pull request as ready for review February 11, 2025 01:50

huerni requested review from RileyWen and Nativu5 February 11, 2025 01:55

RileyWen reviewed Feb 13, 2025

View reviewed changes

protos/PublicDefs.proto Outdated Show resolved Hide resolved

protos/PublicDefs.proto Outdated Show resolved Hide resolved

protos/PublicDefs.proto Outdated Show resolved Hide resolved

RileyWen requested changes Feb 13, 2025

View reviewed changes

huerni mentioned this pull request Feb 14, 2025

Dev/account multi op #382

Closed

huerni linked an issue Feb 14, 2025 that may be closed by this pull request

新增统一管理账号接口 #361

Open

L-Xiafeng added the enhancement New feature or request label Feb 16, 2025

huerni added build Build system change and removed build Build system change labels Feb 17, 2025

huerni requested review from RileyWen and L-Xiafeng February 18, 2025 05:54

huerni commented Feb 20, 2025

View reviewed changes

protos/Crane.proto Outdated Show resolved Hide resolved

L-Xiafeng reviewed Feb 25, 2025

View reviewed changes

L-Xiafeng reviewed Feb 28, 2025

View reviewed changes

huerni force-pushed the dev/account_multi_refactor branch from 86ef8d9 to 4831cd3 Compare March 3, 2025 04:01

L-Xiafeng reviewed Mar 4, 2025

View reviewed changes

src/CraneCtld/AccountManager.cpp Show resolved Hide resolved

huerni force-pushed the dev/account_multi_refactor branch from 4831cd3 to 0e44e23 Compare March 6, 2025 05:48

huerni added 9 commits March 8, 2025 22:14

feat: feat: Multiple errcodes returned.

f032476

refactor

addf65a

refactor: set account/user qos/partition return errcode

73cb720

refactor: query account/user/qos

e449f91

refactor proto and set partition,qos

20fbd83

refactor query

4b90d6c

refactor proto

69d5fad

merge master

e431725

fix: modifyAccount Overwrite

30facb0

huerni added 4 commits March 8, 2025 22:14

refactor

6c1c0cf

feat: set unique

ef17e29

feat: set operator vector to set

36a35a7

fix: block user

f2251d4

RileyWen force-pushed the dev/account_multi_refactor branch from adc6fad to f2251d4 Compare March 8, 2025 14:14

RileyWen reviewed Mar 8, 2025

View reviewed changes

RileyWen marked this pull request as draft March 8, 2025 15:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dev/account multi refactor #426

Dev/account multi refactor #426

huerni commented Jan 24, 2025

RileyWen Feb 13, 2025

huerni Feb 18, 2025

L-Xiafeng Feb 25, 2025

RileyWen Mar 6, 2025

huerni Mar 6, 2025

huerni Mar 6, 2025

L-Xiafeng Mar 6, 2025 •

edited

Loading

huerni Mar 6, 2025

L-Xiafeng Mar 6, 2025

huerni Mar 6, 2025

L-Xiafeng Mar 6, 2025

RileyWen Mar 8, 2025

RileyWen Mar 8, 2025

RileyWen Mar 8, 2025

RileyWen Mar 8, 2025

RileyWen Mar 8, 2025

RileyWen Mar 8, 2025

RileyWen Mar 8, 2025

Dev/account multi refactor #426

Are you sure you want to change the base?

Dev/account multi refactor #426

Conversation

huerni commented Jan 24, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

L-Xiafeng Mar 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

L-Xiafeng Mar 6, 2025 •

edited

Loading