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

[Backport 2.x] Add ApiSpecFetcher for Fetching and Comparing API Specifications #906

Merged
merged 2 commits into from
Oct 9, 2024

Conversation

opensearch-trigger-bot[bot]
Copy link
Contributor

Backport 57b8b59 from #900.

* Added ApiSpecFetcher with test

Signed-off-by: Junwei Dai <junweid@amazon.com>

* remove duplication license

Signed-off-by: Junwei Dai <junweid@amazon.com>

* Add more test to pass test coverage check

Signed-off-by: Junwei Dai <junweid@amazon.com>

* new commit address all comments

Signed-off-by: Junwei Dai <junweid@amazon.com>

* new commit address all comments

Signed-off-by: Junwei Dai <junweid@amazon.com>

* Addressed all comments

Signed-off-by: Junwei Dai <junweid@amazon.com>

---------

Signed-off-by: Junwei Dai <junweid@amazon.com>
Co-authored-by: Junwei Dai <junweid@amazon.com>
(cherry picked from commit 57b8b59)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@dbwiddis
Copy link
Member

dbwiddis commented Oct 8, 2024

CI failing due to Lucene 9.12 upgrade and issues with build. Follow:

dbwiddis
dbwiddis previously approved these changes Oct 8, 2024
@navneet1v
Copy link

@dbwiddis the backport should not because of lucene upgrade. As lucene upgrade has happened only for main branch. :D

@dbwiddis
Copy link
Member

dbwiddis commented Oct 8, 2024

@dbwiddis the backport should not because of lucene upgrade. As lucene upgrade has happened only for main branch. :D

@navneet1v yeah, didn't spend long enough looking at the log, saw NoClassDefFoundError and just assumed.

Seems we need some more dependencies on the backport, related to slf4j (CC: @junweid62)

@dbwiddis dbwiddis self-requested a review October 8, 2024 06:41
@dbwiddis dbwiddis dismissed their stale review October 8, 2024 06:41

needs slf4j import

@dbwiddis
Copy link
Member

dbwiddis commented Oct 9, 2024

Looks like on main we get slf4j.api from rest client

runtimeClasspath - Runtime classpath of source set 'main'.
+--- org.opensearch.client:opensearch-rest-client:3.0.0-SNAPSHOT
|    +--- org.apache.httpcomponents.client5:httpclient5:5.2.1
|    +--- org.apache.httpcomponents.core5:httpcore5:5.2.2 -> 5.3
|    +--- org.apache.httpcomponents.core5:httpcore5-h2:5.2.2
|    +--- commons-codec:commons-codec:1.16.1
|    +--- commons-logging:commons-logging:1.2
|    \--- org.slf4j:slf4j-api:1.7.36

but on 2.x we don't

compileClasspath - Compile classpath for source set 'main'.
+--- org.opensearch.client:opensearch-rest-client:2.18.0-SNAPSHOT
|    +--- org.apache.httpcomponents:httpclient:4.5.14
|    +--- org.apache.httpcomponents:httpcore:4.4.16
|    +--- org.apache.httpcomponents:httpasyncclient:4.1.5
|    +--- org.apache.httpcomponents:httpcore-nio:4.4.16
|    +--- commons-codec:commons-codec:1.16.1
|    +--- commons-logging:commons-logging:1.2
|    +--- io.projectreactor:reactor-core:3.5.20
|    \--- org.reactivestreams:reactive-streams:1.0.4

Signed-off-by: Daniel Widdis <widdis@gmail.com>
Copy link

codecov bot commented Oct 9, 2024

Codecov Report

Attention: Patch coverage is 84.44444% with 7 lines in your changes missing coverage. Please review.

Project coverage is 78.07%. Comparing base (0cde785) to head (d3c8141).
Report is 2 commits behind head on 2.x.

Files with missing lines Patch % Lines
.../opensearch/flowframework/util/ApiSpecFetcher.java 82.05% 4 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                2.x     #906      +/-   ##
============================================
+ Coverage     78.04%   78.07%   +0.03%     
- Complexity      977      995      +18     
============================================
  Files            97       99       +2     
  Lines          4554     4621      +67     
  Branches        423      431       +8     
============================================
+ Hits           3554     3608      +54     
- Misses          823      833      +10     
- Partials        177      180       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dbwiddis dbwiddis merged commit c939b9c into 2.x Oct 9, 2024
31 checks passed
@dbwiddis dbwiddis deleted the backport/backport-900-to-2.x branch October 9, 2024 22:37
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.

3 participants