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

Bypass namespace validator for DescribeNamespace API #3336

Conversation

alexshtin
Copy link
Member

@alexshtin alexshtin commented Sep 8, 2022

What changed?
Bypass namespace validator for DescribeNamespace API.

Why?
DescribeNamespace returns namespace information directly from database bypassing namespace registry. But namespace validator still validates namespace using registry. If there is something wrong with namespace registry it is impossible to read namespace information for debugging.

How did you test it?
Modified existing tests.

Potential risks
No risks.

Is hotfix candidate?
No.

@alexshtin alexshtin requested a review from a team as a code owner September 8, 2022 04:25
return nil, ErrNamespaceNotSet
}
return ni.namespaceRegistry.GetNamespace(namespaceName)
// Special case for DescribeNamespace API which can get namespace by Id and should bypass namespace registry.
Copy link
Member

Choose a reason for hiding this comment

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

update comments: // always pass validation for DescribeNamespace API calls.

@alexshtin alexshtin force-pushed the feature/describe-namespace-bypass-validator branch from ceaf0c5 to 12cea83 Compare September 8, 2022 18:08
@alexshtin alexshtin force-pushed the feature/describe-namespace-bypass-validator branch from 12cea83 to daba691 Compare September 8, 2022 18:11
@alexshtin alexshtin merged commit 57b07b1 into temporalio:master Sep 8, 2022
@alexshtin alexshtin deleted the feature/describe-namespace-bypass-validator branch September 8, 2022 20:12
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