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.

Sign up for GitHub

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

Jump to bottom

feat: 接入 scowd #1193

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

feat: 接入 scowd #1193

wants to merge 34 commits into from

Conversation

Miracle575
Copy link
Contributor

@Miracle575 Miracle575 commented Apr 2, 2024 •
edited

完成 scow 对接 scowd 一期的改造，主要完成 HPC 文件和桌面相关功能改造

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Apr 2, 2024 •
edited

🦋 Changeset detected

Latest commit: d362ae2

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

This PR includes changesets to release 13 packages
Name Type
@scow/portal-server Patch
@scow/scowd-protos Patch
@scow/config Patch
@scow/lib-scowd Patch
@scow/cli Patch
@scow/lib-server Patch
@scow/ai Patch
@scow/audit-server Patch
@scow/auth Patch
@scow/mis-server Patch
@scow/mis-web Patch
@scow/portal-web Patch
@scow/gateway 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

@Miracle575 Miracle575 marked this pull request as ready for review April 18, 2024 02:27
@pkuhpc-review-bot pkuhpc-review-bot bot added the Code-ReviewRequested Code Review Requested label Apr 18, 2024
ddadaal
ddadaal requested changes Apr 18, 2024
View reviewed changes
apps/portal-server/src/services/scowd/desktop.ts Outdated Show resolved Hide resolved
libs/config/src/cluster.ts Outdated Show resolved Hide resolved
apps/cli/src/config/install.ts Outdated Show resolved Hide resolved
apps/portal-server/package.json Outdated Show resolved Hide resolved
apps/portal-server/src/config/env.ts Outdated
@@ -30,6 +30,13 @@ export const config = envConfig({
SSH_PUBLIC_KEY_PATH: str({ desc: "SSH公钥路径", default: join(homedir(), ".ssh", "id_rsa.pub") }),

DOWNLOAD_CHUNK_SIZE: num({ desc: "grpc下载文件时，每个message中的chunk的大小。单位字节", default: 3 * 1024 * 1024 }),

SCOWD_ENABLED: bool({ desc: "是否开启 SCOWD", default: false }),
Copy link
Member

Choose a reason for hiding this comment

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

现在实际上调用的grpc service是多态的，是否启用scowd影响不同的service，但是更好的方式应该是抽象在clusterops部分，apps/portal-server/src/clusterops这里已经为job和app定义了接口，job和app部分的grpc service会调用clusterops部分的逻辑，不同的cluster可以用不同的clusterops逻辑。这样可以支持有的cluster启用了scowd，有的没有的场景

apps/portal-server/src/utils/scowd.ts Outdated
Comment on lines 48 to 49
// 函数将 Code 转换为 @grpc/grpc-js 的 status
export function convertCodeToGrpcStatus(code: Code): status {
Copy link
Member

Choose a reason for hiding this comment

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

这个居然connectrpc没有提供？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个翻了一下文档，只看到 转为 http 的 status code 的，没找到转为 grpc 的

Copy link
Member

Choose a reason for hiding this comment

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

可以去对比一下，connect的code和grpc的code定义是一模一样的，不需要转换

libs/config/src/cluster.ts Outdated Show resolved Hide resolved
@pkuhpc-review-bot pkuhpc-review-bot bot added Code-ChangeRequested and removed Code-ReviewRequested Code Review Requested labels Apr 18, 2024
@Miracle575 Miracle575 requested a review from ddadaal April 25, 2024 07:54
@pkuhpc-review-bot pkuhpc-review-bot bot added Code-ReviewRequested Code Review Requested and removed Code-ChangeRequested labels Apr 25, 2024
ddadaal
ddadaal requested changes Apr 25, 2024
View reviewed changes
Copy link
Member

@ddadaal ddadaal left a comment •
edited

Choose a reason for hiding this comment

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

写scowd相关的配置文档，可以先写在飞书里看看

apps/portal-server/src/clusterops/api/file.ts Outdated Show resolved Hide resolved
apps/portal-server/src/utils/scowd.ts Outdated
Comment on lines 48 to 49
// 函数将 Code 转换为 @grpc/grpc-js 的 status
export function convertCodeToGrpcStatus(code: Code): status {
Copy link
Member

Choose a reason for hiding this comment

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

可以去对比一下，connect的code和grpc的code定义是一模一样的，不需要转换

apps/portal-server/src/clusterops/desktop/scowdDesktop.ts Outdated Show resolved Hide resolved
@pkuhpc-review-bot pkuhpc-review-bot bot added Code-ChangeRequested and removed Code-ReviewRequested Code Review Requested labels Apr 25, 2024
@Miracle575 Miracle575 requested a review from ddadaal April 26, 2024 07:59
@pkuhpc-review-bot pkuhpc-review-bot bot added Code-ReviewRequested Code Review Requested and removed Code-ChangeRequested labels Apr 26, 2024
ddadaal
ddadaal requested changes Apr 27, 2024
View reviewed changes
apps/portal-server/src/utils/scowd.ts Outdated
Comment on lines 43 to 45
export const getScowdClient = (cluster: string) => {
return scowdClientForClusters[cluster];
};
Copy link
Member

Choose a reason for hiding this comment

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

如果所有getScowdClient的调用都会在返回undefined的时候报错，可以把异常直接写在这个函数里，即使不是，也需要显式指定返回值为可能是undefined的，以免调用者忘记处理undefined的情况。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

考虑直接从这个函数中抛出异常

libs/scowd/src/ssl.ts Outdated Show resolved Hide resolved
@pkuhpc-review-bot pkuhpc-review-bot bot added Code-ChangeRequested and removed Code-ReviewRequested Code Review Requested labels Apr 27, 2024
@Miracle575 Miracle575 requested a review from ddadaal April 28, 2024 09:46
@pkuhpc-review-bot pkuhpc-review-bot bot added Code-ReviewRequested Code Review Requested and removed Code-ChangeRequested labels Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ddadaal ddadaal Awaiting requested review from ddadaal

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
Code-ReviewRequested Code Review Requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Miracle575 @ddadaal