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

Update ES client to 7.16-canary.7 #117305

Merged
merged 34 commits into from
Nov 4, 2021
Merged

Conversation

mshustov
Copy link
Contributor

@mshustov mshustov commented Nov 3, 2021

Summary

closes #117285

@mshustov mshustov added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:skip Skip the PR/issue when compiling release notes labels Nov 3, 2021
@mshustov mshustov marked this pull request as ready for review November 4, 2021 08:40
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@@ -87,7 +87,7 @@ export const fetchNumericFieldStats = async (
);
const { body } = await esClient.search(request);

const aggregations = body.aggregations as {
const aggregations = body.aggregations as unknown as {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

tsc complains sample doesn't exist on Record<string, AggregationsAggregate>. We need to refactor the way elasticsearch-js declares `aggregations type. A first proposal is here elastic/elasticsearch-js#1596

Copy link
Member

Choose a reason for hiding this comment

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

Very likely this new aggregations work will land only in v8.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it can create some merge conflicts during a bug fix backporting, but let's see

@@ -315,11 +315,11 @@ export async function bulkUpdateAgents(
});

return {
items: res.body.items.map((item: estypes.BulkResponseItemContainer) => ({
// @ts-expect-error ErrorCause is not assignable to Error
items: res.body.items.map((item) => ({
Copy link
Contributor Author

@mshustov mshustov Nov 4, 2021

Choose a reason for hiding this comment

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

I'm not sure these typings are correct. @joshdover

Copy link
Contributor

Choose a reason for hiding this comment

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

@nchaulet I believe you're more familiar with this code. Should we be wrapping the item.update!.error with new Error()?

Copy link
Member

Choose a reason for hiding this comment

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

Looking at the code looks like we never consume these errors, I am going to investigate a little more what error handling we are missing, but it should not block that PR.

@mshustov
Copy link
Contributor Author

mshustov commented Nov 4, 2021

@elasticmachine merge upstream

@@ -85,11 +85,13 @@ export const initLogSourceConfigurationRoutes = ({ framework, sources }: InfraBa
? sources.updateSourceConfiguration(
requestContext.core.savedObjects.client,
sourceId,
// @ts-ignore
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've already added the same instructions during #113950
The problem is that an interface incompatibility error is highly unstable. scripts/type_check might fail locally, but pass on CI

@mshustov
Copy link
Contributor Author

mshustov commented Nov 4, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
kibana 845 928 +83

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@mshustov mshustov merged commit bd0ef1e into elastic:7.16 Nov 4, 2021
@mshustov mshustov deleted the update-es-client branch November 4, 2021 15:48
@mshustov mshustov changed the title Update ES client to 7.16-canary.6 Update ES client to 7.16-canary.7 Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Feature:elasticsearch release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.16.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants