-
Notifications
You must be signed in to change notification settings - Fork 18
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
Dev/account multi op #382
base: master
Are you sure you want to change the base?
Dev/account multi op #382
Conversation
huerni
commented
Dec 3, 2024
- block/unblock account 支持批量操作账户
- block/unblock user 支持同一账户下批量操作用户
- 添加、删除账户/用户的partition、qos 支持批量操作
- 查询账户/用户 支持批量操作
- 删除账户/用户 支持批量操作
src/CraneCtld/AccountManager.cpp
Outdated
std::unordered_map<std::string, Account>* res_account_map) { | ||
User res_user; | ||
CraneExpected<void> result{}; | ||
|
||
std::vector<std::string> account_vec = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个在前端分好吧 后端就不要字符串操作了 字符串操作尽量在前端就做好 后端只处理结构化数据
src/CraneCtld/AccountManager.cpp
Outdated
if (!user.account_to_attrs_map.at(account).allowed_partition_qos_map.contains( | ||
partition)) | ||
return std::unexpected(CraneErrCode::ERR_ALLOWED_PARTITION); | ||
std::vector<std::string> partition_vec = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这边为啥还是有好多StrSplit
src/CraneCtld/AccountManager.cpp
Outdated
@@ -1826,12 +1965,17 @@ AccountManager::CraneExpected<void> AccountManager::AddUserAllowedPartition_( | |||
|
|||
User res_user(user); | |||
|
|||
std::vector<std::string> partition_vec = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个地方在前端split完吧 不然后端这边参数语义太模糊了
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个是因为value是共用的,延续之前的写法,改成list的话,modify这一块可能都得改成list
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我又看了一下 确实挺多的,还重复split很多,我改成前端split吧,然后看看其他还有什么问题,没问题改完就可以给周老师测试了