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

Delete unused parts of rest-api-spec #4027

Open
dblock opened this issue Jul 27, 2022 · 4 comments
Open

Delete unused parts of rest-api-spec #4027

dblock opened this issue Jul 27, 2022 · 4 comments
Labels
enhancement Enhancement or improvement to existing feature or request

Comments

@dblock
Copy link
Member

dblock commented Jul 27, 2022

Is your feature request related to a problem? Please describe.

The actual spec portion of https://github.com/opensearch-project/OpenSearch/tree/main/rest-api-spec is AFAIK unused, and likely incorrect. Looking at, for example, 931813f, we update the tests, but we don't make changes to the spec.

Describe the solution you'd like
Delete any unused parts.

Additional context
More recent work in https://github.com/opensearch-project/opensearch-api-specification is probably the future of API spec, authored in Smithy. The idea is that that repo will become the canonical source of truth for versioned APIs, that we will build and publish it into OpenAPI, and that we would generate clients and server from it.

@dblock dblock added enhancement Enhancement or improvement to existing feature or request untriaged labels Jul 27, 2022
@dblock dblock changed the title Delete rest-api-spec Delete unused parts of rest-api-spec Jul 27, 2022
@dblock
Copy link
Member Author

dblock commented Jul 27, 2022

@reta Is what I am saying ^ accurate? I was inspired by your change there.

@reta
Copy link
Collaborator

reta commented Jul 27, 2022

👍 @dblock you are very right, the spec seems to be out of sync and needs cleanup (it also references ealsticsearch all over the place) ....

@andrross
Copy link
Member

Out of curiosity I did some poking here and found that

rm -rf ./rest-api-spec/src/main/resources/rest-api-spec/api

will at least result in a failure when running:

./gradlew ':client:rest-high-level:test' --tests "org.opensearch.client.RestHighLevelClientTests.testApiNamingConventions"

so the spec isn't entirely unused. Just figured I'd share for anyone else like me who is unfamiliar with this :)

@VachaShah
Copy link
Collaborator

The tests in this spec are also used in opensearch-rs for test cases https://github.com/opensearch-project/opensearch-rs/blob/main/yaml_test_runner/src/github.rs#L73 and in api-generator https://github.com/opensearch-project/opensearch-rs/blob/main/api_generator/src/rest_spec/mod.rs#L47.

There is a discussion to decouple the tests in rust client from this rest-api-spec. Refer opensearch-project/opensearch-rs#59 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request
Projects
None yet
Development

No branches or pull requests

4 participants