Skip to content
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

Refactored the src and test of GeoHashGrid and GeoTileGrid Aggregations on GeoPoint from server folder to geo module.(#4071) (#4072) #4180

Merged
merged 1 commit into from
Aug 22, 2022

Conversation

navneet1v
Copy link
Contributor

Description

Refactored the src and test of GeoHashGrid and GeoTileGrid Aggregations on GeoPoint from server folder to geo module.(#4071) (#4072)

The changes also include:

  • Updated Search plugin to provide the interface so that plugins can also register the composite aggregations
  • Added YAML test for the geo_grid, geo_tile and composite aggregation

Signed-off-by: Navneet Verma [email protected]

Issues Resolved

(#4071) (#4072)

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

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.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2022

Gradle Check (Jenkins) Run Completed with:

@navneet1v
Copy link
Contributor Author

Gradle Check (Jenkins) Run Completed with:

Failing because of some docker issue on another Azure plugin. I have created the issue with the build team: opensearch-project/opensearch-build#2446

Once it is fixed, I will re-trigger the Jenkins build step.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@navneet1v
Copy link
Contributor Author

Gradle Check (Jenkins) Run Completed with:

14:48 /Volumes/workplace/OpenSearch bucket-aggregation$ ./gradlew ':server:test' --tests "org.opensearch.search.PitMultiNodeTests.testCreatePitWhileNodeDropWithAllowPartialCreationFalse" -Dtests.seed=5FA0956A2A8702B6 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ro-RO -Dtests.timezone=Asia/Pyongyang
Picked up JAVA_TOOL_OPTIONS: -Dlog4j2.formatMsgNoLookups=true

> Configure project :qa:os
Cannot add task 'destructiveDistroTest.docker' as a task with that name already exists.
=======================================
OpenSearch Build Hamster says Hello!
  Gradle Version        : 7.5
  OS Info               : Mac OS X 12.5 (x86_64)
  JDK Version           : 17 (Amazon Corretto JDK)
  JAVA_HOME             : /Users/navneev/.sdkman/candidates/java/17.0.3.6.1-amzn
  Random Testing Seed   : 5FA0956A2A8702B6
  In FIPS 140 mode      : false
=======================================

> Task :server:compileJava
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

> Task :test:framework:compileJava
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

> Task :server:compileTestJava
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

> Task :server:test
Picked up JAVA_TOOL_OPTIONS: -Dlog4j2.formatMsgNoLookups=true
OpenJDK 64-Bit Server VM warning: Ignoring option --illegal-access=warn; support was removed in 17.0
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.opensearch.bootstrap.BootstrapForTesting (file:/Volumes/workplace/OpenSearch/test/framework/build/distributions/framework-3.0.0-SNAPSHOT.jar)
WARNING: Please consider reporting this to the maintainers of org.opensearch.bootstrap.BootstrapForTesting
WARNING: System::setSecurityManager will be removed in a future release
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.gradle.api.internal.tasks.testing.worker.TestWorker (file:/Users/navneev/.gradle/wrapper/dists/gradle-7.5-all/6qsw290k5lz422uaf8jf6m7co/gradle-7.5/lib/plugins/gradle-testing-base-7.5.jar)
WARNING: Please consider reporting this to the maintainers of org.gradle.api.internal.tasks.testing.worker.TestWorker
WARNING: System::setSecurityManager will be removed in a future release

BUILD SUCCESSFUL in 1m 41s
42 actionable tasks: 16 executed, 1 from cache, 25 up-to-date

The same test is working successful on the local.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Copy link
Collaborator

@nknize nknize left a comment

Choose a reason for hiding this comment

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

Failure looks like a known issue? (#4162)

./gradlew ':server:internalClusterTest' --tests "org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderIT.testRestoreSnapshotAllocationDoesNotExceedWatermark" -Dtests.seed=683FC95AAA3080A9 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=en-IN -Dtests.timezone=America/Santarem -Druntime.java=17

I was unable to repro locally. Refiring to not hold up the PR.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

…ns on GeoPoint from server folder to geo module.(opensearch-project#4071) (opensearch-project#4072)

The changes also include:
   * Updated Search plugin to provide the interface so that plugins can also register the compositie aggregations
   * Added YAML test for the geo_grid, geo_tile and composite aggregation

Signed-off-by: Navneet Verma <[email protected]>
@navneet1v
Copy link
Contributor Author

Failure looks like a known issue? (#4162)

./gradlew ':server:internalClusterTest' --tests "org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderIT.testRestoreSnapshotAllocationDoesNotExceedWatermark" -Dtests.seed=683FC95AAA3080A9 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=en-IN -Dtests.timezone=America/Santarem -Druntime.java=17

I was unable to repro locally. Refiring to not hold up the PR.

Pulled and rebase to refire the gradle checks

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

Codecov Report

Merging #4180 (da7dbea) into main (4643620) will decrease coverage by 0.00%.
The diff coverage is 86.04%.

@@             Coverage Diff              @@
##               main    #4180      +/-   ##
============================================
- Coverage     70.68%   70.67%   -0.01%     
- Complexity    57142    57219      +77     
============================================
  Files          4606     4607       +1     
  Lines        274839   275022     +183     
  Branches      40241    40277      +36     
============================================
+ Hits         194273   194385     +112     
- Misses        64282    64411     +129     
+ Partials      16284    16226      -58     
Impacted Files Coverage Δ
...ava/org/opensearch/client/RestHighLevelClient.java 44.02% <ø> (-0.31%) ⬇️
...egations/bucket/composite/GeoTileValuesSource.java 50.00% <ø> (ø)
...aggregations/bucket/geogrid/BoundedCellValues.java 100.00% <ø> (ø)
...gregations/bucket/geogrid/BucketPriorityQueue.java 75.00% <ø> (ø)
...arch/aggregations/bucket/geogrid/CellIdSource.java 69.23% <ø> (ø)
...search/aggregations/bucket/geogrid/CellValues.java 100.00% <ø> (ø)
...ions/bucket/geogrid/GeoGridAggregationBuilder.java 75.67% <ø> (ø)
...aggregations/bucket/geogrid/GeoGridAggregator.java 81.25% <ø> (ø)
.../bucket/geogrid/GeoHashGridAggregationBuilder.java 100.00% <ø> (ø)
...egations/bucket/geogrid/GeoHashGridAggregator.java 80.00% <ø> (ø)
... and 524 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@navneet1v
Copy link
Contributor Author

@nknize can you please merge the pull request too.

@nknize nknize merged commit 5433056 into opensearch-project:main Aug 22, 2022
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x 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-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-4180-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 54330560a7064434e088dad0eeb61ad0df2cfa6b
# Push it to GitHub
git push --set-upstream origin backport/backport-4180-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-4180-to-2.x.

navneet1v added a commit to navneet1v/OpenSearch that referenced this pull request Aug 22, 2022
…ns on GeoPoint from server folder to geo module.(opensearch-project#4071) (opensearch-project#4072) (opensearch-project#4180)

The changes also includes:
   * Updated Search plugin to provide the interface so that plugins can also register the composite aggregations
   * Added YAML test for the geo_grid, geo_tile and composite aggregation

Signed-off-by: Navneet Verma <[email protected]>
@navneet1v
Copy link
Contributor Author

navneet1v commented Aug 22, 2022

The backport to 2.x failed:

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

To backport manually, run these commands in your terminal:

BASH (SHELL) (16 lines)

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

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-4180-to-2.x.

Doing the manual backport using #4281

nknize pushed a commit that referenced this pull request Aug 30, 2022
…ns on GeoPoint from server folder to geo module.(#4071) (#4072) (#4180) (#4281)

The changes also includes:
   * Updated Search plugin to provide the interface so that plugins can also register the composite aggregations
   * Added YAML test for the geo_grid, geo_tile and composite aggregation

Signed-off-by: Navneet Verma <[email protected]>
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.

4 participants