-
Notifications
You must be signed in to change notification settings - Fork 61
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
Added distributions/included and excluded. #510
Added distributions/included and excluded. #510
Conversation
e1d1844
to
68f92ac
Compare
Changes AnalysisCommit SHA: a11b293 API ChangesSummary
ReportThe full API changes report is available at: https://github.com/opensearch-project/opensearch-api-specification/actions/runs/10390880736/artifacts/1812191323 API Coverage
|
Signed-off-by: dblock <[email protected]>
68f92ac
to
fdebe95
Compare
Spec Test Coverage Analysis
|
Signed-off-by: dblock <[email protected]>
f116d93
to
c198ca9
Compare
… against Amazon OpenSearch 2.13. Signed-off-by: dblock <[email protected]>
c198ca9
to
a11b293
Compare
@@ -291,6 +291,9 @@ paths: | |||
operationId: cat.nodeattrs.0 | |||
x-operation-group: cat.nodeattrs | |||
x-version-added: '1.0' | |||
x-distributions-excluded: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dblock coming from opensearch-project/opensearch-java#1152 (comment), I understand the bias towards supporting AWS, but I honestly believe the spec has to be vendor neutral. I believe it should be feasible to keep the distribution specific details outside of the spec and allow the distribution specific spec to be produced if needed:
For example:
- the vendor neutral spec:
openapi-oss.yaml
- the AWS managed spec:
openapi-aws.yaml
(would only contain new APIs, excluded APIs or APIs that are tailored)
The final spec for AWS: openapi-oss.yaml
+ openapi-aws.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can/should open an issue copy-pasting the above about removing x-distribution-excluded.
When I introduced this feature I did not intend it to be biased towards supporting AWS in any way, but I did want to run the tests against Amazon Managed so I annotated some APIs. We would welcome other keywords here, be it oracle-oci
, aiven-managed
or something platform-specific like community-freebsd
in the short term. Therefore I believe the spec is vendor netural today. I think it's ok to mention vendors in the spec this way, but we can talk about it.
What you are looking at is the source for the spec, not the spec. We currently publish the all-versions spec in a release that includes these x-
options. But this repo is capable of producing a version-specific spec (e.g. 1.3.x, without APIs added in/after 2.0 for example or vice-versa) (see https://github.com/opensearch-project/opensearch-api-specification/blob/main/DEVELOPER_GUIDE.md#spec-merger), and that could be extended to produce a vendor-less (by stripping x-distribution
options), or a vendor-specific spec (e.g. -amazon-managed.yaml
).
Description
Following up on #483, added
distributions
withincluded
andexcluded
. I prefer to manage exclusions in tests so that a potential new distribution can run this with all tests enabled the first time, which makes it possible to immediately see what is broken.Removed
undefined
values from results, and use an exact comparison in integration tests, making it much easier to see what scenarios failed.Passing
npm run test:spec -- --opensearch-distribution=amazon-managed
against Amazon Managed OpenSearch 2.13. The values ofssl_provider_http
,ssl_cipher
andssl_protocol
are coming backnull
fromsslinfo
and most security APIs are disabled.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.