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

Update Multi-Region Namespace APIs #29

Merged
merged 4 commits into from
May 14, 2024
Merged

Update Multi-Region Namespace APIs #29

merged 4 commits into from
May 14, 2024

Conversation

yux0
Copy link
Contributor

@yux0 yux0 commented May 14, 2024

What was changed

Update Multi-Region Namespace APIs

Why?

Update Multi-Region Namespace APIs

Checklist

  1. Closes

  2. How was this tested:

  1. Any docs updates needed?

yux0 and others added 2 commits May 14, 2024 11:52
Co-authored-by: Chad Retz <[email protected]>
Co-authored-by: Chad Retz <[email protected]>
@yux0 yux0 requested a review from mastermanu May 14, 2024 18:53
}

message AddNamespaceRegionRequest {
// The namespace to add region to.
Copy link
Member

Choose a reason for hiding this comment

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

"The namespace to add the region to"

message AddNamespaceRegionRequest {
// The namespace to add region to.
string namespace = 1;
// The id of the region to add to the namespace.
Copy link
Member

Choose a reason for hiding this comment

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

Do we need any clarification comment here that this is secondary/passive region?

}

message NamespaceRegionStatus {
// The current state of the namespace region.
Copy link
Member

Choose a reason for hiding this comment

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

[Nit] might want to reword this a bit as this could be interpreted as "the state of the whole region the namespace is in"
Maybe, the current state of the namespace in the given region or something would be better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the state of a region where the namespace is in.

@yux0 yux0 merged commit da16fe0 into main May 14, 2024
3 checks passed
@yux0 yux0 deleted the mrn-v2 branch May 14, 2024 22:34
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.

3 participants