Skip to content

Start of fixes for core/search #1947

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

Merged
merged 2 commits into from
Oct 11, 2022
Merged

Start of fixes for core/search #1947

merged 2 commits into from
Oct 11, 2022

Conversation

sethmlarson
Copy link
Contributor

@sethmlarson sethmlarson commented Oct 6, 2022

Part of #1878

@sethmlarson sethmlarson force-pushed the fix-types-core-search branch from c4fb915 to ebd6572 Compare October 7, 2022 02:49
@sethmlarson sethmlarson requested a review from swallez October 10, 2022 11:48
@sethmlarson sethmlarson force-pushed the fix-types-core-search branch from 622cfa0 to 0f35a3b Compare October 10, 2022 12:57
@sethmlarson sethmlarson requested a review from swallez October 10, 2022 13:00
Copy link
Member

@swallez swallez left a comment

Choose a reason for hiding this comment

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

Looks good so far!

@@ -49,7 +49,7 @@ export class ErrorCause

export class ShardFailure {
index?: IndexName
node?: string
node?: string | null
Copy link
Member

Choose a reason for hiding this comment

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

Does null for this field exist in the recordings? I can't relate this to one of the Java client errors (which actually doesn't error out on null for optional fields).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this was in one of the recordings not a fix for Java specifically.

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 went looking for the test this is from and couldn't find it so reverted this change for now.

@sethmlarson sethmlarson force-pushed the fix-types-core-search branch from 0f35a3b to 0c4848b Compare October 10, 2022 17:29
@sethmlarson sethmlarson force-pushed the fix-types-core-search branch from 0c4848b to 43effe8 Compare October 10, 2022 19:44
@github-actions
Copy link
Contributor

Following you can find the validation results for the APIs you have changed.

API Status Request Response
field_caps 🔴 50/51 51/51
search 🔴 1552/1583 1562/1565

You can validate these APIs yourself by using the make validate target.

Copy link
Member

@swallez swallez left a comment

Choose a reason for hiding this comment

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

LGTM

@swallez swallez merged commit dacbd3c into main Oct 11, 2022
@swallez swallez deleted the fix-types-core-search branch October 11, 2022 13:53
@github-actions
Copy link
Contributor

The backport to 7.17 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-7.17 7.17
# Navigate to the new working tree
cd .worktrees/backport-7.17
# Create a new branch
git switch --create backport-1947-to-7.17
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick --mainline 1 dacbd3cf71ea2b3331169befe0e82d72c5a06e52
# Push it to GitHub
git push --set-upstream origin backport-1947-to-7.17
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-7.17

Then, create a pull request where the base branch is 7.17 and the compare/head branch is backport-1947-to-7.17.

github-actions bot pushed a commit that referenced this pull request Oct 11, 2022
swallez pushed a commit that referenced this pull request Oct 11, 2022
swallez pushed a commit that referenced this pull request Oct 11, 2022
swallez added a commit that referenced this pull request Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants