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

feat(portal): 提交作业和提交交互式应用时显示可用分区 #977

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

piccaSun
Copy link
Contributor

@piccaSun piccaSun commented Nov 14, 2023

做了什么

修改提交作业和提交交互式应用时账户及分区获取逻辑
修改为选择的账户为用户可以使用的正常未封锁账户(账户状态未封锁,用户在账户下状态未封锁)
选择账户时获取该账户的可用分区并短暂保存
当切换集群或刷新账户或刷新分区的时候会重新发起请求获取新的数据

修改后提交作业页面

1.不使用模板值时
账户,分区,QOS为可选列表的第一项
image
集群,账户,分区发生变更时,重置下级关联项为可选列表第一项
image
image
2.使用模板值时无论是否仍为当前可选列表中的值都可以直接填入
image
集群出现错误时会在页面直接报错
image
账户出现错误时当前为允许提交但是作业无法正常运行
分区等信息出现错误时会直接报错
image
使用模板值时,集群,账户,分区任意值发生变化时,下级关联项目将重置为可选列表的第一项,
节点数gpu卡数等内容正常开始校验

修改后创建交互式应用页面

  1. 没有上一次提交记录时
    打开页面账户,分区,qos分别为可选列表的第一项
    image
    适当选择填入不同值
    image
    上一级条件发生变更时下一级条件重置为可选列表第一项,gpu卡数或节点数更新为默认值1
    image

2.当上一次提交记录存在
如果仍然为当前可选列表值则打开页面后正常填入
如果不可选则自动更新为可选列表的第一项
之后动作与1相同

Copy link

changeset-bot bot commented Nov 14, 2023

🦋 Changeset detected

Latest commit: 394f0b5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 15 packages
Name Type
@scow/portal-server Patch
@scow/mis-server Patch
@scow/portal-web Patch
@scow/mis-web Patch
@scow/grpc-api Minor
@scow/protos Patch
@scow/lib-hook Patch
@scow/lib-operation-log Patch
@scow/lib-scheduler-adapter Patch
@scow/lib-server Patch
@scow/audit-server Patch
@scow/ai Patch
@scow/auth Patch
@scow/gateway Patch
@scow/cli Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

codecov bot commented Nov 14, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.28%. Comparing base (39f0c33) to head (2bc30b5).
Report is 27 commits behind head on master.

❗ Current head 2bc30b5 differs from pull request most recent head 394f0b5. Consider uploading reports for the commit 394f0b5 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #977      +/-   ##
==========================================
- Coverage   68.53%   68.28%   -0.26%     
==========================================
  Files         145      134      -11     
  Lines        4354     3998     -356     
  Branches      597      535      -62     
==========================================
- Hits         2984     2730     -254     
+ Misses       1239     1170      -69     
+ Partials      131       98      -33     

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

@piccaSun piccaSun marked this pull request as ready for review December 4, 2023 11:02
@piccaSun piccaSun requested a review from ddadaal December 4, 2023 11:02
Comment on lines 52 to 54
* This API function GetAvailablePartitions has been deprecated.
* Use the new API function GetAvailablePartitionsForCluster from ./config/configServiceServer instead.
* @deprecated
Copy link
Member

Choose a reason for hiding this comment

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

deprecate的描述应该用一行写在@deprecated 后面

const filterAccountPromise = Promise.allSettled(accounts.map(async (account) => {
try {
const resp = await asyncClientCall(client.account, "queryAccountBlockStatus", { accountName: account });
resp.blocked ? filteredBlockedAccounts.push(account) : filteredUnblockedAccounts.push(account);
Copy link
Member

Choose a reason for hiding this comment

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

这里不用表达式的值就用正常的if吧。

Comment on lines 88 to 92
await Promise.all([filterAccountPromise, filterUserStatusPromise]).then(() => {
unblockAccounts = filteredUnblockedAccounts.filter((account) =>
filteredUnblockedUserAccounts.includes(account));
blockedAccounts = Array.from(new Set(filteredBlockedAccounts.concat(filteredBlockedUserAccounts)));
});
Copy link
Member

Choose a reason for hiding this comment

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

此处可以不用then,then中的内容直接作为下一条statement。

unblockedAccounts和blockedAccounts这两个变量可以在此处定义,不用再前面预先定义

accountName: account, userId });
resp.blocked ? filteredBlockedUserAccounts.push(account) : filteredUnblockedUserAccounts.push(account);
} catch (error) {
logger.error(`Error occured when query the block status of ${userId} in ${account}.`);
Copy link
Member

Choose a reason for hiding this comment

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

把error打印出来

@piccaSun
Copy link
Contributor Author

piccaSun commented Dec 5, 2023

@ddadaal 上述问题已修改,请确认

Comment on lines 2 to 5
"@scow/portal-server": minor
"@scow/mis-server": minor
"@scow/portal-web": minor
"@scow/mis-web": minor
Copy link
Member

Choose a reason for hiding this comment

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

确认一下这个功能是minor还是major

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ddadaal 已确认,不需要到major,根据API版本变动升级到minor就可以了

Copy link
Member

Choose a reason for hiding this comment

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

不是 我本来的意思是patch😂确认了是minor也行

Comment on lines 2 to 5
"@scow/portal-server": minor
"@scow/mis-server": minor
"@scow/portal-web": minor
"@scow/mis-web": minor
Copy link
Member

Choose a reason for hiding this comment

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

不是 我本来的意思是patch😂确认了是minor也行

@piccaSun
Copy link
Contributor Author

piccaSun commented Dec 6, 2023

grpc-api的版本Level无需与scow一致,所以经确认修改功能上等级为patch

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.

None yet

2 participants