-
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
[BWC and API enforcement] Decorate the existing APIs with proper annotations (part 3) #11182
Conversation
❌ Gradle check result for 9b20bce: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Compatibility status:Checks if related components are compatible with change 199a68d Incompatible componentsIncompatible components: [https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/performance-analyzer.git] Skipped componentsCompatible componentsCompatible components: [https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/geospatial.git] |
❕ Gradle check result for 7de9a36: 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. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #11182 +/- ##
============================================
+ Coverage 71.09% 71.26% +0.17%
- Complexity 58752 58865 +113
============================================
Files 4888 4888
Lines 277207 277222 +15
Branches 40282 40287 +5
============================================
+ Hits 197077 197561 +484
+ Misses 63654 63130 -524
- Partials 16476 16531 +55 ☔ View full report in Codecov by Sentry. |
326a566
to
318a704
Compare
❕ Gradle check result for 326a566: 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. |
❕ Gradle check result for 61bd2d3: 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. |
❕ Gradle check result for 3a34bb6: 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. |
❌ Gradle check result for 72f2f4f: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for 72f2f4f: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❕ Gradle check result for 72f2f4f: 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. |
@andrross @dblock @nknize looks like the last large chunk, there are only 2 issues to fix (one of them should be addressed by #10921):
|
❌ Gradle check result for 6eca3b7: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for 7d1515d: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❕ Gradle check result for b64aae8: 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. |
@reta Unfortunately the other PR I just merged caused a conflict here :( |
❕ Gradle check result for 07ccc2e: 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. |
…tations (part 3) Signed-off-by: Andriy Redko <[email protected]>
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-11182-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 09bacee5fc85676e97bee6b4ad87dec35c6aa8cc
# Push it to GitHub
git push --set-upstream origin backport/backport-11182-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x Then, create a pull request where the |
…tations (part 3) (opensearch-project#11182) Signed-off-by: Andriy Redko <[email protected]> (cherry picked from commit 09bacee)
…tations (part 3) (#11182) (#11231) Signed-off-by: Andriy Redko <[email protected]> (cherry picked from commit 09bacee)
@@ -276,7 +282,7 @@ public Parameter<T> setValidator(Consumer<T> validator) { | |||
/** | |||
* Configure a custom serializer for this parameter | |||
*/ | |||
public Parameter<T> setSerializer(Serializer<T> serializer, Function<T, String> conflictSerializer) { | |||
protected Parameter<T> setSerializer(Serializer<T> serializer, Function<T, String> conflictSerializer) { |
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.
@reta I saw this failed for k-NN plugin field mapper: https://github.com/opensearch-project/k-NN/blob/main/src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapper.java#L138. Should Serializer be public? It seems like it is similar to setValidator.
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.
Thanks @jmazanec15 , I will take a look shortly, the Serializer
is defined as protected interface
@InternalApi
protected interface Serializer<T> {
void serialize(XContentBuilder builder, String name, T value) throws IOException;
}
so it was really weird to have a public method that uses the protected interface
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.
@jmazanec15 the change was reverted, should be all good, thank you for noticing!
…tations (part 3) (opensearch-project#11182) Signed-off-by: Andriy Redko <[email protected]>
…tations (part 3) (opensearch-project#11182) Signed-off-by: Andriy Redko <[email protected]>
…tations (part 3) (opensearch-project#11182) Signed-off-by: Andriy Redko <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>
Description
Decorate the existing APIs with proper annotations. This is merely the first part so we could work through the small changesets vs big bang pull request. The API validations are fully automated (as annotation processor #9304).
All classes that are annotated in this pull request leak through
org.opensearch.client.Client
which is by definition (currently) is public APIs exposed through plugins and RestClient / HighLevelRestClient.Related Issues
Part of #9303
Check List
New functionality includes testing.All tests passNew functionality has been documented.New functionality has javadoc addedCommit changes are listed out in CHANGELOG.md file (See: Changelog)Public documentation issue/PR createdBy 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.