-
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
Mark StepListener as @opensearch.api #17076
Mark StepListener as @opensearch.api #17076
Conversation
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
❕ Gradle check result for 95242a0: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
*/ | ||
|
||
@PublicApi(since = "2.19.0") |
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.
Just a thought here. Since StepListener
has been used in Plugins before. Should we keep this is "1.0.0", basically since inception?
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.
Changed to 1.0.0
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17076 +/- ##
============================================
- Coverage 72.27% 72.14% -0.14%
+ Complexity 65336 65239 -97
============================================
Files 5301 5301
Lines 303824 303824
Branches 44033 44033
============================================
- Hits 219587 219179 -408
- Misses 66249 66638 +389
- Partials 17988 18007 +19 ☔ View full report in Codecov by Sentry. |
@cwperks if we don't expose this class as public API (primarily, through |
Signed-off-by: Craig Perkins <[email protected]>
Its not exposed
Not that I know of, but I noticed it is prevalently used across plugins. It is a form of ActionListener which is marked as |
If it is not exposed, this is not a public API (the fact plugins use it does not mean we have to make it public to wage the maintenance cost) |
I guess I'm confused about what |
It 100% does mean that: this means the API is internal to OpenSearch core, could be changed any time without notice, and if used, is solely the responsibility of the user, no guarantees from the core. |
Got it. This looks to me like a class that should have been marked |
As you mentioned, we don't expose it. If we don't expose it, it is internal, as designed and marked at the moment (sadly, nothing prevents anyone from using this class since is is |
Description
Opening up this small PR to mark the StepListener as
@opensearch.api
due to its current use across a few plugins in the default distribution: https://github.com/search?q=org%3Aopensearch-project%20StepListener&type=codeCheck List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.