-
Notifications
You must be signed in to change notification settings - Fork 8k
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
Upgrade re2 from v1.17.7 to v1.20.1 #162880
Conversation
Pinging @elastic/kibana-operations (Team:Operations) |
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.
Pulled and working. Just one quick question.
"win32-x64" | ||
) | ||
node_api_versions=( 108 115 ) | ||
formats=( "br" "gz" ) |
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.
What is the br
file used for? Wondering if we are using both.
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.
I was thinking the same thing. Both existed in bucket already, so I just did the same thing this time around. But I actually don't think we need it. Should I remove it in this PR or do y'all prefer that we do that at a later stage? I think there's a bunch of stuff regarding how we store these files that probably needs to be "cleaned up" now that we also have the custom build of Node.js and re2 in another bucket as well
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.
I did a quick search for anything using br
and didn't find anything. Maybe best to leave it for now and do a more thorough check in the clean up pass.
Missed in review, sorry. Think we need to add |
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @watson |
💔 All backports failed
Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
(cherry picked from commit 4c41247) # Conflicts: # package.json # src/dev/build/tasks/patch_native_modules_task.ts # yarn.lock
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
💔 All backports failed
Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
Friendly reminder: Looks like this PR hasn’t been backported yet. |
1 similar comment
Friendly reminder: Looks like this PR hasn’t been backported yet. |
I was generating new
re2
binaries to work with Node.js API version 115 (i.e. Node.js 20), so I thought I might as well bump the version used inmain
to the newest one while I was at it.I've uploaded the relevant artifacts for both Node.js API version 115 and version 108 (i.e. Node.js 18) to a new folder named
1.20.1
in theyarn-prebuilt-artifacts
bucket. So this should work both with the current Node.js 18 and also once we upgrade to Node.js 20 (in which case only the URL's and SHASUMs should be updated insrc/dev/build/tasks/patch_native_modules_task.ts
).This PR also introduces
scripts/download_re2.sh
to make it easier to upgradere2
in the future.Tasks
re2