-
Notifications
You must be signed in to change notification settings - Fork 360
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
add support for namespace search-attributes #604
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Dariusch Ochlast <dariusch.ochlast@gmail.com>
|
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.
Search attributes do not belong to namespaces on self-hosted Temporal.
I'm happy to include this feature, but it needs to be separated from any namespace related code.
What do you mean? Afaik according to the docs they are namespace scoped. I would be happy to change the PR if you could point me in the right direction. |
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.
My apologies, this has changed since I last used it. They are indeed scoped to namespaces. One small change needed though.
Co-authored-by: Rob Holland <rob.holland@gmail.com>
Also now learned that Elasticsearch implementation does not have per namespace search attributes, only SQL does. How do you think best to handle that? |
@robholland I added a condition, comment, and tests only to render the search attributes for the SQL driver |
What was changed
I am adding support for adding search attributes while creating a namespace.
Why?
We recently migrated to the temporal chart and missed the option to include the previously used config.
Checklist
values:
output:
No, its an optional opt-in.