-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[BUG][DISTRIBUTION][2.0.0] org.gradle.api.GradleException: Compilation error #2858
Comments
@peterzhuamazon Just to confirm, is this pulling from the main branch? I am not able to replicate this issue. Can you provide more details on how to replicate this? |
@CEHENKLE @bbarani I have tested alerting assemble with both last thursday snapshot as well as latest snapshot. The crash only happens on latest snapshot which indicate the core changes cause this issue. I see there are a lot of changes between April 7 and today. https://github.com/opensearch-project/OpenSearch/commits/2.0 Thanks. |
@peterzhuamazon Just to confirm my understanding. there are 3 PRs merged since April 7th. So, this issue could be caused by at least one of them. couldn't it? |
I see a lot of commits within this period. @anasalkouz I think one or more of this cause the issue. Thanks. |
This is tied to the stack overflow fix. Looks like the |
The simplest possible mitigation would be to change the code in
The ideal mitigation would be to have the alerting plugin adopt the new API signature so we can correctly impose the reg-ex limit and protect against the stack overflow bug. I'm not familiar with the alerting plugin codebase so I cannot speak to how feasible this is. |
@lezzago ^^ |
copying my comment from #2810 (comment)
|
@kartg ^^ |
Working on a mitigation now. The quickest way forward would be to decouple |
sounds good to me, thanks! |
@rishabhmaurya PR is up, please take a look. Also, is there a mechanism (that I'm unaware of) to identify classes or APIs such as these are not an explicit Opensearch core API but used outside the engine's code base? I'd like to explore what we can do to ensure this compatibility break doesn't recur. |
there is no direct way to identify, one way is to search github globally or at least within the opensearch organization for consumers. |
FIx has been merged into Backport PRs for |
@opensearch-project/opensearch-core @dblock @nknize Ooof. That's really bad. Do folks have other thoughts on how we could get ahead of breaks like this? |
Hi @kartg seems like it is fixed now I see alerting passed in pipeline. Thanks. |
@CEHENKLE As @rishabhmaurya the modularization work (#1838) is probably the right long-term way to address this. The strong encapsulation offered by Java modules should prevent this. |
@andrross Sure -- but I don't know of any active work on that project (that doesn't mean it isn't happening, it just means I haven't seen it percolating around), and I don't wanna get pantsed on the regular while we wait. /C |
if we are concerned about late detection of breaking changes among plugins, then plugins integrating with nightly builds of core could be another way to detect and fix them sooner. |
The text was updated successfully, but these errors were encountered: