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

[Feature/extensions] Communication mechanism JS (Part 1) #5586

Closed
wants to merge 6 commits into from
Closed

[Feature/extensions] Communication mechanism JS (Part 1) #5586

wants to merge 6 commits into from

Conversation

vibrantvarun
Copy link
Member

@vibrantvarun vibrantvarun commented Dec 15, 2022

Description

Request Response model to fetch Job details like job index and job type from extensions.

Issues Resolved

opensearch-project/opensearch-sdk-java#274

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
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

Companion PR

opensearch-sdk-java opensearch-project/opensearch-sdk-java#289
anomaly detection opensearch-project/anomaly-detection#761

Sequence to merge PR's

  1. OpenSearch
  2. opensearch-sdk-java
  3. anomaly detection

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

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

Codecov Report

Merging #5586 (df2bd3a) into feature/extensions (c7c908e) will increase coverage by 0.08%.
The diff coverage is 14.70%.

❗ Current head df2bd3a differs from pull request most recent head 6c2401b. Consider uploading reports for the commit 6c2401b to get more accurate results

@@                   Coverage Diff                    @@
##             feature/extensions    #5586      +/-   ##
========================================================
+ Coverage                 70.92%   71.00%   +0.08%     
- Complexity                58504    58585      +81     
========================================================
  Files                      4767     4770       +3     
  Lines                    279189   279256      +67     
  Branches                  40316    40318       +2     
========================================================
+ Hits                     198001   198289     +288     
+ Misses                    65058    64770     -288     
- Partials                  16130    16197      +67     
Impacted Files Coverage Δ
...ain/java/org/opensearch/extensions/JobDetails.java 0.00% <0.00%> (ø)
.../org/opensearch/extensions/JobDetailsResponse.java 0.00% <0.00%> (ø)
...ensearch/extensions/JobDetailsResponseHandler.java 0.00% <0.00%> (ø)
...a/org/opensearch/extensions/ExtensionsManager.java 60.92% <58.82%> (-0.34%) ⬇️
...pensearch/action/ingest/DeletePipelineRequest.java 31.25% <0.00%> (-37.50%) ⬇️
...a/org/opensearch/index/mapper/MapperException.java 75.00% <0.00%> (-25.00%) ⬇️
...ensearch/client/indices/DetailAnalyzeResponse.java 20.54% <0.00%> (-24.66%) ⬇️
...ensearch/search/profile/query/CollectorResult.java 75.34% <0.00%> (-24.66%) ⬇️
...nsearch/index/shard/IndexingOperationListener.java 63.93% <0.00%> (-22.96%) ⬇️
...pensearch/index/mapper/MapperParsingException.java 77.77% <0.00%> (-22.23%) ⬇️
... and 485 more

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

@joshpalis joshpalis self-requested a review December 15, 2022 22:25
@joshpalis
Copy link
Member

joshpalis commented Dec 15, 2022

Thanks Varun for implementing the initial request/response workflow to enable Job Scheduling for extensions. I would suggest that you add a unit test to test the behavior of the JobDetailsResponse and JobDetailsResponseHandler.

Here is an example that you can refer to:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.cluster.allocation.AwarenessAllocationIT.testThreeZoneOneReplicaWithForceZoneValueAndLoadAwareness

Copy link
Member

@owaiskazi19 owaiskazi19 left a comment

Choose a reason for hiding this comment

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

Needs tests

@saratvemulapalli saratvemulapalli changed the title Communication mechanism JS (Part 1) [Feature/extensions] Communication mechanism JS (Part 1) Dec 16, 2022
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants