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 schema elements that don't match target OpenSearch version. #428

Merged
merged 13 commits into from
Jul 26, 2024

Conversation

dblock
Copy link
Member

@dblock dblock commented Jul 16, 2024

Description

This change allows us to generates a merged spec that targets a specific version without all the unnecessary parts. It accomplishes a number of goals.

  1. It removes fields from the spec in integration testing that have been x-version-removed before the target server version or x-version-added after, effectively testing that the values of these fields are correct.
  2. It enables generating specs for a given version of OpenSearch without branching. We could publish a separate 1.x spec and a 2.x spec instead of 1 combined spec, removing the burden of having to check x-version fields from consumers of the spec. This would allow us to maintain stable releases of clients that are only compatible with OpenSearch 1.x for example with only fixes, without adding any 2.x methods to them.
npm run merge -- --opensearch-version=1.3 --output build/v1.3.yaml
npm run merge -- --opensearch-version=2.0 --output build/v2.0.yaml
diff build/v1.3.yaml build/v2.0.yaml

This removes non-matching parameters.

132a134
>         - $ref: '#/components/parameters/indices.put_alias::query.cluster_manager_timeout'

and entire operations

>     cat.templates::query.cluster_manager_timeout:
>       name: cluster_manager_timeout
>       in: query
>       description: Operation timeout for connection to cluster-manager node.
>       schema:
>         $ref: '#/components/schemas/_common:Duration'
>       x-version-added: '2.0'

Issues Resolved

Closes #425.

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.

Copy link
Contributor

github-actions bot commented Jul 16, 2024

Changes Analysis

Commit SHA: 7cf2517
Comparing To SHA: 6fd9afe

API Changes

Summary

NO CHANGES

Report

The full API changes report is available at: https://github.com/opensearch-project/opensearch-api-specification/actions/runs/10104069658/artifacts/1742071927

API Coverage

Before After Δ
Covered (%) 500 (48.97 %) 500 (48.97 %) 0 (0 %)
Uncovered (%) 521 (51.03 %) 521 (51.03 %) 0 (0 %)
Unknown 24 24 0

@dblock dblock marked this pull request as ready for review July 16, 2024 21:21
@dblock dblock force-pushed the test-version branch 4 times, most recently from c86af16 to fcbedee Compare July 16, 2024 21:40
@dblock dblock marked this pull request as draft July 16, 2024 21:48
@dblock dblock force-pushed the test-version branch 2 times, most recently from 9bc5996 to 5edc5ba Compare July 18, 2024 12:55
@dblock dblock marked this pull request as ready for review July 18, 2024 13:00
@dblock dblock force-pushed the test-version branch 2 times, most recently from 2bd2fd6 to c0d194b Compare July 18, 2024 17:26
@dblock dblock force-pushed the test-version branch 3 times, most recently from 36b5cee to 47e5737 Compare July 18, 2024 22:49
@dblock dblock requested a review from nhtruong July 18, 2024 22:53
@dblock dblock force-pushed the test-version branch 2 times, most recently from cc915a2 to 19e41ee Compare July 22, 2024 18:05
Copy link
Contributor

github-actions bot commented Jul 22, 2024

Spec Test Coverage Analysis

Total Tested
524 118 (22.52 %)

@dblock dblock force-pushed the test-version branch 2 times, most recently from 7136d8a to 06c7feb Compare July 23, 2024 20:32
@dblock dblock marked this pull request as draft July 23, 2024 23:09
@dblock dblock marked this pull request as ready for review July 24, 2024 14:54
@dblock
Copy link
Member Author

dblock commented Jul 24, 2024

@nhtruong I think I addressed all the feedback. The updated implementation will delete all refs that don't match semver, then purge all unused refs and finally purge all objects that reference all refs that were removed for the entire resulting spec.

tools/src/merger/OpenApiVersionExtractor.ts Outdated Show resolved Hide resolved
tools/src/merger/OpenApiVersionExtractor.ts Outdated Show resolved Hide resolved
tools/src/helpers.ts Outdated Show resolved Hide resolved
tools/src/helpers.ts Show resolved Hide resolved
@dblock dblock marked this pull request as draft July 25, 2024 23:29
@dblock dblock marked this pull request as ready for review July 26, 2024 01:22
@dblock dblock requested a review from nhtruong July 26, 2024 01:23
@nhtruong nhtruong merged commit aec3145 into opensearch-project:main Jul 26, 2024
16 checks passed
@dblock dblock deleted the test-version branch July 26, 2024 22:51
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.

[FEATURE] Remove properties with x-version-added/removed when checking schema
2 participants