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

Merge feature/openapi/final into Main #208

Merged
merged 4 commits into from
Apr 1, 2024
Merged

Conversation

nhtruong
Copy link
Collaborator

@nhtruong nhtruong commented Mar 26, 2024

Switching over to native OpenAPI:

  • Removed Smithy:
    • Removed Smithy models (model folder)
    • Removed Gradle
    • Removed legacy test framework
    • Removed build-openapi-specs workflow
  • Added Editable OpenAPI spec:
    • Added multi-file OpenApi spec (spec folder)
    • Added a tool to merge multi-file spec into single-file spec
    • Added build-single-file-specs workflow
  • Updated Documents

Adding Request and Response Bodies

The OpenAPI spec has also been updated with the request and response bodies that we extracted from ElasticSearch OpenAPI spec. This is done within the same PR as the native OpenAPI switch because we also used the ElasticSearch OpenAPI spec to map the query/path schemas to the schemas of many components in the bodies.


Reviewers: It's easier to review this PR like reviewing a new repo by inspecting the feature branch


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.

- Removed Smithy:
  - Removed Smithy models (`model` folder)
  - Removed Gradle
  - Removed legacy test framework
  - Removed build-openapi-specs workflow
- Added Editable OpenAPI spec:
  - Added multi-file OpenApi spec (`spec` folder)
  - Added a tool to merge multi-file spec into single-file spec
  - Added build-single-file-specs workflow
- Updated Documents
## Adding Request and Response Bodies

The OpenAPI spec has also been updated with the request and response bodies that we extracted from ElasticSearch OpenAPI spec. This is done within the same PR as the native OpenAPI switch because we also used the ElasticSearch OpenAPI spec to map the query/path schemas to the schemas of many components in the bodies.

Signed-off-by: Theo Truong <theotr@amazon.com>
dblock
dblock previously approved these changes Mar 27, 2024
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Looked quickly over the code and the specs, looks good! I'd like to discuss checking in the output spec vs. publishing it as a draft "latest" release, but we can do that later.

.github/workflows/coverage.yml Outdated Show resolved Hide resolved
tools/merger/merge.ts Show resolved Hide resolved
@dblock
Copy link
Member

dblock commented Mar 27, 2024

@Xtansia take a look, merge?

Signed-off-by: Theo Truong <theotr@amazon.com>
Copy link
Collaborator

@Xtansia Xtansia left a comment

Choose a reason for hiding this comment

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

Everything looks good generally, I have a similar concern to @dblock regarding checking in the built spec, but agree it can wait until after. Only issue I noticed was that there's a couple remaining references to xpack that should be tidied up

spec/schemas/nodes.info.yaml Outdated Show resolved Hide resolved
Signed-off-by: Theo Truong <theotr@amazon.com>
Signed-off-by: Theo Truong <theotr@amazon.com>
@nhtruong
Copy link
Collaborator Author

nhtruong commented Apr 1, 2024

@dblock the API Coverage workflow is still being triggered from main branch. We will have to disable the workflow in the repo settings to get this merged.

@dblock
Copy link
Member

dblock commented Apr 1, 2024

@dblock the API Coverage workflow is still being triggered from main branch. We will have to disable the workflow in the repo settings to get this merged.

That's ok.

@dblock
Copy link
Member

dblock commented Apr 1, 2024

@Xtansia good with you?

@Xtansia Xtansia merged commit cd96458 into main Apr 1, 2024
4 of 5 checks passed
@Xtansia Xtansia deleted the feature/openapi/final branch April 1, 2024 20:12
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