-
Notifications
You must be signed in to change notification settings - Fork 609
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(SBC): Provision proto #20452
feat(SBC): Provision proto #20452
Conversation
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.
proto/sbc_provision.proto
Outdated
string label = 1; | ||
} | ||
|
||
service NodeGroupController { |
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.
Shall we unify the term, such as ResourceGroupController
?
@shanicky PTAL
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.
Do we plan to provision something other than node groups? If not we can also be precise and use NodeGroupController
.
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.
I'm fine with both. Resource group
is what's used in the kernel, while node group
is from the operator. They do have a 1-1 mapping.
…o cajan93/sbc-proto
I will add the Proto-related changes in https://github.com/risingwavelabs/serverless-backfill-controller/pull/81 |
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.
LGTM; it aligns with the RFC, but please keep the SBC-related code as clean as possible in the future.
proto/sbc_provision.proto
Outdated
syntax = "proto3"; | ||
|
||
// Provision a node via the serverless backfill controller. | ||
package sbc; |
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.
Could you use a full name? Future colleagues might not immediately associate "sbc" with anything.
…o cajan93/sbc-proto
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
Communication between kernel and the Serverless Backfill Controller (SBC). Proto definitions should be in this repo and not in the SBC repo.
Equivalent on SBC side: https://github.com/risingwavelabs/serverless-backfill-controller/pull/76
Checklist
Documentation
Release note