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.

Sign up for GitHub

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

Jump to bottom

Add ApiSpecFetcher for Fetching and Comparing API Specifications #900

Merged
merged 7 commits into from
Oct 8, 2024
Merged

Add ApiSpecFetcher for Fetching and Comparing API Specifications #900

merged 7 commits into from
Oct 8, 2024

Conversation

junweid62
Copy link
Contributor

Description

  • Introduced a new ApiSpecFetcher utility to programmatically fetch and compare OpenAPI specifications.

  • Supports comparing required input fields between an OpenAPI spec and predefined enum values.

  • The current implementation serves as a foundational mechanism, with the comparison logic successfully integrated into the Register Agent workflow.

  • Future PRs will extend this mechanism to other workflow steps, ensuring consistent validation across the API specification.

Related Issues

Part of #651

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

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.

Added ApiSpecFetcher with test
f2a0c4e 
Signed-off-by: Junwei Dai <junweid@amazon.com>
@github-actions github-actions bot added the backport 2.x backport PRs to 2.x branch label Oct 4, 2024
remove duplication license
65b5303 
Signed-off-by: Junwei Dai <junweid@amazon.com>
@codecov Codecov
Copy link

codecov bot commented Oct 4, 2024 •
edited
Loading

Codecov Report

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

Project coverage is 78.10%. Comparing base (7a93d6c) to head (af15a00).
Report is 1 commits behind head on main.

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              @@
##               main     #900      +/-   ##
============================================
+ Coverage     78.04%   78.10%   +0.06%     
- Complexity      977      990      +13     
============================================
  Files            97       99       +2     
  Lines          4554     4599      +45     
  Branches        423      428       +5     
============================================
+ Hits           3554     3592      +38     
- Misses          823      827       +4     
- Partials        177      180       +3

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

Add more test to pass test coverage check
87a23a9 
Signed-off-by: Junwei Dai <junweid@amazon.com>
dbwiddis
dbwiddis reviewed Oct 5, 2024
View reviewed changes
Copy link
Member

@dbwiddis dbwiddis left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks great! A few suggestions you can take or leave; only thing I'd insist on changing before approval is the too-broad throwing of Exception.

CHANGELOG.md Show resolved Hide resolved
build.gradle Outdated Show resolved Hide resolved
build.gradle Outdated Show resolved Hide resolved
build.gradle Outdated Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/exception/ApiSpecParseException.java Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java Outdated Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java Outdated Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java Outdated Show resolved Hide resolved
src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java Outdated Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java Outdated Show resolved Hide resolved
owaiskazi19
owaiskazi19 reviewed Oct 7, 2024
View reviewed changes
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.

Great start!

src/main/java/org/opensearch/flowframework/exception/ApiSpecParseException.java Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java Outdated Show resolved Hide resolved
src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java Outdated Show resolved Hide resolved
src/test/java/org/opensearch/flowframework/workflow/RegisterAgentTests.java Show resolved Hide resolved
junweid62 and others added 2 commits October 7, 2024 11:43
dbwiddis
dbwiddis reviewed Oct 7, 2024
View reviewed changes
Copy link
Member

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

A few more tweaks.

build.gradle Outdated Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/common/CommonValue.java Outdated Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java Outdated Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java Outdated Show resolved Hide resolved
src/test/java/org/opensearch/flowframework/exception/ApiSpecParseExceptionTests.java Outdated Show resolved Hide resolved
new commit address all comments
f021ef8 
Signed-off-by: Junwei Dai <junweid@amazon.com>
dbwiddis
dbwiddis approved these changes Oct 8, 2024
View reviewed changes
Copy link
Member

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

Approving with a few last items to clean up

src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java Outdated Show resolved Hide resolved
src/test/java/org/opensearch/flowframework/util/ApiSpecFetcherTests.java Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/util/ApiSpecFetcher.java Outdated Show resolved Hide resolved
Addressed all comments
af15a00 
Signed-off-by: Junwei Dai <junweid@amazon.com>
@owaiskazi19 owaiskazi19 merged commit 57b8b59 into opensearch-project:main Oct 8, 2024
20 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 8, 2024
@github-actions
Add ApiSpecFetcher for Fetching and Comparing API Specifications (#900)
6344ff7 
* 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>
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Oct 8, 2024
Merged
@junweid62 junweid62 mentioned this pull request Oct 8, 2024
Closed
7 tasks
dbwiddis added a commit that referenced this pull request Oct 9, 2024
@opensearch-trigger-bot @github-actions @dbwiddis
[Backport 2.x] Add ApiSpecFetcher for Fetching and Comparing API Spec…
c939b9c 
…ifications (#906)

* Add ApiSpecFetcher for Fetching and Comparing API Specifications (#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>

* Add slf4j-api and jackson-core dependencies

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

---------

Signed-off-by: Junwei Dai <junweid@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Junwei Dai <junweid@amazon.com>
Co-authored-by: Daniel Widdis <widdis@gmail.com>
@owaiskazi19 owaiskazi19 mentioned this pull request Oct 9, 2024
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@owaiskazi19 owaiskazi19 owaiskazi19 approved these changes

@dbwiddis dbwiddis dbwiddis approved these changes

@joshpalis joshpalis Awaiting requested review from joshpalis joshpalis is a code owner

@ohltyler ohltyler Awaiting requested review from ohltyler ohltyler is a code owner

@amitgalitz amitgalitz Awaiting requested review from amitgalitz amitgalitz is a code owner

@jackiehanyang jackiehanyang Awaiting requested review from jackiehanyang jackiehanyang is a code owner

Assignees
No one assigned
Labels
backport 2.x backport PRs to 2.x branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@junweid62 @dbwiddis @owaiskazi19