8000 Exclude deleted namespaces from ListNamespace response by alexshtin · Pull Request #2646 · temporalio/temporal · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Exclude deleted namespaces from ListNamespace response #2646

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

Merged
merged 3 commits into from
Mar 26, 2022

Conversation

alexshtin
Copy link
Contributor
@alexshtin alexshtin commented Mar 25, 2022

What changed?
Exclude deleted namespaces from ListNamespace response.

Why?
By default deleted namespaces (which exist in database during reclaim resources step) should not be returned by ListNamespace API.

How did you test it?
New unit tests.

Potential risks
No risks.

Is hotfix candidate?
No.

@alexshtin alexshtin requested a review from a team as a code owner March 25, 2022 18:00
// do not include the metadata record
if name != namespaceMetadataRecordName {
// do not include the metadata record
if name == namespaceMetadataRecordName {
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 file changes are not related to deleted namespace but it had the same problem (essentially bug). If there was namespaceMetadataRecordName record in response, results might have fewer items than defined by pageSize. It is easy testable if pageSize set to 1. Previous implementation just returned empty page in the middle of pagination.

if skippedRows == 0 {
break
}
pageSize = skippedRows
Copy link
Member

Choose a reason for hiding this comment

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

this is clever.

@alexshtin alexshtin force-pushed the feature/list-namespace-deleted branch from 2e18547 to 5fc0a5f Compare March 25, 2022 23:39
@alexshtin
Copy link
Contributor Author

API changes is here: temporalio/api#167

@alexshtin alexshtin enabled auto-merge (squash) March 25, 2022 23:44
@alexshtin alexshtin merged commit 752ff50 into temporalio:master Mar 26, 2022
@alexshtin alexshtin deleted the feature/list-namespace-deleted branch March 26, 2022 00:00
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.

2 participants
0