Allow registering search attributes without Advance Visibility enabled #5185
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…nfig - Remove validation for Advance Visibility Store - Add Advance Visibility Config check before update ElasticSearch/OpenSearch mapping - Remove co-related test for 'no advanced config'
Update CHANGELOG.md
|
|
lancezhao-ins
changed the title
|
lgtm. Can you also test it with https://github.com/uber-common/cadence-samples/tree/master/cmd/samples/recipes/searchattributes ? |
service/frontend/adminHandler.go
Outdated
| if adh.esClient.IsNotFoundError(err) { | ||
| err = adh.params.ESClient.CreateIndex(ctx, index) | ||
| // when have valid advance visibility config, update elasticsearch mapping, new added field will not be able to remove or update | ||
| if err := adh.validateConfigForAdvanceVisibility(); err == nil { |
There was a problem hiding this comment.
The only downside of this approach is that NOOP will never be logged/reported, maybe add warn level message when validation is failing?
There was a problem hiding this comment.
Added warning "Skip updating OpenSearch/ElasticSearch mapping since Advance Visibility hasn't been enabled." when Advance Visibility is not enabled.
Passed using |
longquanzheng
approved these changes
Apr 4, 2023
davidporter-id-au
added a commit
that referenced
this pull request
Apr 12, 2023
commit eade936 Author: David Porter <david.porter@uber.com> Date: Wed Apr 12 13:54:13 2023 -0700 Corrects the config-store handling for not-found errors (#5203) commit 9fc4485 Author: lancezhao-ins <99238165+lancezhao-ins@users.noreply.github.com> Date: Tue Apr 11 04:21:15 2023 +1000 Allow registering search attributes without Advance Visibility enabled (#5185) * remove validation & test for add search attribute with no advanced config - Remove validation for Advance Visibility Store - Add Advance Visibility Config check before update ElasticSearch/OpenSearch mapping - Remove co-related test for 'no advanced config' * Update CHANGELOG.md Update CHANGELOG.md * Add warn level message if skip updating OpenSearch/ElasticSearch mapping * Add warn level message and add validSearchAttributes in development.yaml --------- Co-authored-by: Quanzheng Long <prclqz@gmail.com> commit d165c7b Author: neil-xie <104041627+neil-xie@users.noreply.github.com> Date: Mon Apr 10 10:46:04 2023 -0700 Update idls version (#5200) commit b2bc8bf Author: Mantas Šidlauskas <mantass@netapp.com> Date: Mon Apr 10 20:12:53 2023 +0300 Add thin ES clients (#5162) * Add thin ES clients
davidporter-id-au
added a commit
that referenced
this pull request
Apr 13, 2023
commit eade936 Author: David Porter <david.porter@uber.com> Date: Wed Apr 12 13:54:13 2023 -0700 Corrects the config-store handling for not-found errors (#5203) commit 9fc4485 Author: lancezhao-ins <99238165+lancezhao-ins@users.noreply.github.com> Date: Tue Apr 11 04:21:15 2023 +1000 Allow registering search attributes without Advance Visibility enabled (#5185) * remove validation & test for add search attribute with no advanced config - Remove validation for Advance Visibility Store - Add Advance Visibility Config check before update ElasticSearch/OpenSearch mapping - Remove co-related test for 'no advanced config' * Update CHANGELOG.md Update CHANGELOG.md * Add warn level message if skip updating OpenSearch/ElasticSearch mapping * Add warn level message and add validSearchAttributes in development.yaml --------- Co-authored-by: Quanzheng Long <prclqz@gmail.com> commit d165c7b Author: neil-xie <104041627+neil-xie@users.noreply.github.com> Date: Mon Apr 10 10:46:04 2023 -0700 Update idls version (#5200) commit b2bc8bf Author: Mantas Šidlauskas <mantass@netapp.com> Date: Mon Apr 10 20:12:53 2023 +0300 Add thin ES clients (#5162) * Add thin ES clients Merge issue
davidporter-id-au
added a commit
that referenced
this pull request
Apr 19, 2023
commit b18be27 Author: Mantas Šidlauskas <mantass@netapp.com> Date: Tue Apr 18 14:12:09 2023 +0300 Add generic ES query building utilities (#5168) commit 824f0ac Author: agautam478 <72432016+agautam478@users.noreply.github.com> Date: Fri Apr 14 13:37:29 2023 -0700 Fixed the nil pointer issues in the InactiveDomain Invariant (#5213) commit c5678dd Author: Ketsia <115650494+ketsiambaku@users.noreply.github.com> Date: Wed Apr 12 15:21:10 2023 -0700 Fix consistent query metric (#5170) * add shardid tag to log * remove counter for overall scope * fix lint commit eade936 Author: David Porter <david.porter@uber.com> Date: Wed Apr 12 13:54:13 2023 -0700 Corrects the config-store handling for not-found errors (#5203) commit 9fc4485 Author: lancezhao-ins <99238165+lancezhao-ins@users.noreply.github.com> Date: Tue Apr 11 04:21:15 2023 +1000 Allow registering search attributes without Advance Visibility enabled (#5185) * remove validation & test for add search attribute with no advanced config - Remove validation for Advance Visibility Store - Add Advance Visibility Config check before update ElasticSearch/OpenSearch mapping - Remove co-related test for 'no advanced config' * Update CHANGELOG.md Update CHANGELOG.md * Add warn level message if skip updating OpenSearch/ElasticSearch mapping * Add warn level message and add validSearchAttributes in development.yaml --------- Co-authored-by: Quanzheng Long <prclqz@gmail.com> commit d165c7b Author: neil-xie <104041627+neil-xie@users.noreply.github.com> Date: Mon Apr 10 10:46:04 2023 -0700 Update idls version (#5200) commit b2bc8bf Author: Mantas Šidlauskas <mantass@netapp.com> Date: Mon Apr 10 20:12:53 2023 +0300 Add thin ES clients (#5162) * Add thin ES clients
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changed?
Why?
It's convenient to test the search attribute API in the workflow code before using advanced visibility.
How did you test it?
Tested locally with AdvancedVisibility Enabled and AdvancedVisibility Disabled
Potential risks
Release notes
Allow registering search attributes when Advanced Visibility is not enabled
Documentation Changes