-
Notifications
You must be signed in to change notification settings - Fork 960
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
[Backport 2.x] [MD] Support legacy client for data source (#2204) #2484
[Backport 2.x] [MD] Support legacy client for data source (#2204) #2484
Conversation
* support legacy client for data source * not wrap 401 error for data source client Signed-off-by: Su <szhongna@amazon.com> (cherry picked from commit 746b9df)
Changelog check failed because previous PR has included this commit's changelog |
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.
@zhongnansu For a backport PR, we're mostly concerned with backwards compatibility. I have just one question about what looks like a change to an existing API in the data plugin.
|
||
constructor(callDataCluster: LegacyAPICaller | OpenSearchClient) { |
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.
Is changing the valid call signature of this class backwards compatible? Is it possible it would break any plugins attempting to call the IndexPatternsFetcher
with anOpenSearchClient
? Or was this change only introduced as part of the initial MD PR?
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.
yeah it's mostly a follow-up to the initial MD PR. At that time, since legacy client is not supported by MD, but index_pattern_fetcher is still using legacy client. We had a workaround to enforce index_pattern_fetcher to use new Client when talking to data sources, use legacy client when talk to default cluster. Now with the support of legacy client, we can get rid of the workaround, and keep the type declaration clean, to only use legacy client for any scenario
…search-project#2484) * support legacy client for data source * not wrap 401 error for data source client Signed-off-by: Su <szhongna@amazon.com> (cherry picked from commit 746b9df)
…search-project#2484) * support legacy client for data source * not wrap 401 error for data source client Signed-off-by: Su <szhongna@amazon.com> (cherry picked from commit 746b9df)
Backport commit 746b9df from #2204