Skip to content

Delete schema elements that don't match target OpenSearch version.#428

Merged
nhtruong merged 13 commits intoopensearch-project:mainfrom
dblock:test-version
Jul 26, 2024
Merged

Delete schema elements that don't match target OpenSearch version.#428
nhtruong merged 13 commits intoopensearch-project:mainfrom
dblock:test-version

Conversation

@dblock
Copy link
Copy Markdown
Member

@dblock dblock commented Jul 16, 2024 •
edited
Loading

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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jul 16, 2024 •
edited
Loading

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
nhtruong
nhtruong reviewed Jul 18, 2024
View reviewed changes
Comment thread tools/src/merger/OpenApiMerger.ts Outdated
nhtruong
nhtruong reviewed Jul 18, 2024
View reviewed changes
Comment thread tools/src/merger/OpenApiMerger.ts Outdated
nhtruong
nhtruong reviewed Jul 18, 2024
View reviewed changes
Comment thread tools/src/merger/OpenApiMerger.ts Outdated
@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
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jul 22, 2024 •
edited
Loading

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
Copy Markdown
Member Author

dblock commented Jul 24, 2024 •
edited
Loading

@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.

nhtruong
nhtruong reviewed Jul 24, 2024
View reviewed changes
Comment thread tools/src/merger/OpenApiVersionExtractor.ts Outdated
Comment thread tools/src/merger/OpenApiVersionExtractor.ts Outdated
Comment thread tools/src/helpers.ts Outdated
Comment thread tools/src/helpers.ts
nhtruong
nhtruong reviewed Jul 24, 2024
View reviewed changes
Comment thread tools/src/merger/OpenApiVersionExtractor.ts Outdated
dblock added 10 commits July 25, 2024 17:13
@dblock
Delete schema elements that don't match target OpenSearch version.
f3277f7 
Signed-off-by: dblock <dblock@amazon.com>
@dblock
Got rid of merge().
7508a3d 
Signed-off-by: dblock <dblock@amazon.com>
@dblock
Extract delete_matching_keys into a helper.
f39f2e8 
Signed-off-by: dblock <dblock@amazon.com>
@dblock
Split up OpenApiVersionExtractor from OpenApiMerger.
f77e76c 
Signed-off-by: dblock <dblock@amazon.com>
@dblock
Removed unused refs.
cdf777d 
Signed-off-by: dblock <dblock@amazon.com>
@dblock
Simplify deletes.
6d176b5 
Signed-off-by: dblock <dblock@amazon.com>
@dblock
Generalize removing unused references.
d81dec7 
Signed-off-by: dblock <dblock@amazon.com>
@dblock
Pick and extend to dry.
02eb409 
Signed-off-by: dblock <dblock@amazon.com>
@dblock
Remove any paths that had all their verbs removed.
203550b 
Signed-off-by: dblock <dblock@amazon.com>
@dblock
Require target_version.
74cdcde 
Signed-off-by: dblock <dblock@amazon.com>
@dblock
Return a Set from find_refs.
e661b32 
Signed-off-by: dblock <dblock@amazon.com>
@dblock
Traverse $ref's recursively.
ed18c4d 
Signed-off-by: dblock <dblock@amazon.com>
@dblock dblock marked this pull request as draft July 25, 2024 23:29
@dblock
Fix: circular references.
7cf2517 
Signed-off-by: dblock <dblock@amazon.com>
@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
@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

Reviewers

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

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

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

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

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

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

1 more reviewer

@nhtruong nhtruong nhtruong approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

2 participants

@dblock @nhtruong