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

Fixed missing global params #280

Merged
merged 1 commit into from
May 3, 2024

Conversation

nhtruong
Copy link
Collaborator

@nhtruong nhtruong commented May 3, 2024

This bug happens when the original name of the params in _global_parameters.yaml are identical to the names they are to be generated in the result file, which has the form of _global::query.{name}. During the generation the original name was removed and replaced by the global name. Swapping the order of key deleting and assignment solved this. Also updated the names in the _global_parameters.yaml to reflect that the merger will handle the naming convention to keep this file simple.

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.

Signed-off-by: Theo Truong <theotr@amazon.com>
@nhtruong nhtruong force-pushed the fix_missing_global_params branch from 51f64d6 to 03a45f4 Compare May 3, 2024 13:58
Copy link

github-actions bot commented May 3, 2024

API specs implemented for 266/649 (40%) APIs.

1 similar comment
Copy link

github-actions bot commented May 3, 2024

API specs implemented for 266/649 (40%) APIs.

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.

This should have a test, but won't hold the PR for the fix.

@dblock dblock merged commit a8822aa into opensearch-project:main May 3, 2024
6 checks passed
@nhtruong nhtruong deleted the fix_missing_global_params branch May 3, 2024 21:12
@nhtruong
Copy link
Collaborator Author

nhtruong commented May 3, 2024

@dblock There are tests that assure that the global params are add. This was just a very edge case that only happens for this very specific situation during the transition to having the global params in their own file.

@dblock
Copy link
Member

dblock commented May 4, 2024

@nhtruong You'll agree that no tests covered this case, this or another edge case can reintroduce the problem.

djmadeira added a commit to djmadeira/opensearch-api-specification that referenced this pull request May 7, 2024
additional paths

Fixed spec (opensearch-project#246)

Signed-off-by: saimedhi <saimedhi@amazon.com>

Fixed search_pipeline spec (opensearch-project#253)

Signed-off-by: saimedhi <saimedhi@amazon.com>

Updated API name: search_pipeline.create to search_pipeline.put (opensearch-project#254)

Signed-off-by: saimedhi <saimedhi@amazon.com>

Fixed search_pipeline.get spec (opensearch-project#255)

Signed-off-by: saimedhi <saimedhi@amazon.com>

Filled in Missing Defaults (opensearch-project#249)

* Filled in Missing Defaults

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

* # Wordings

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

---------

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

Replace pull_request_target workflow with two workflows that upload/download an artifact. (opensearch-project#251)

* Fix: replace pull_request_target with a download/upload artifact.

Signed-off-by: dblock <dblock@amazon.com>

* Use upload/download-artifact@v4.

Signed-off-by: dblock <dblock@amazon.com>

---------

Signed-off-by: dblock <dblock@amazon.com>

Fix: var -> const. (opensearch-project#258)

Signed-off-by: dblock <dblock@amazon.com>

Adds tools linter. (opensearch-project#260)

Signed-off-by: dblock <dblock@amazon.com>

Fix: '@typescript-eslint/indent': 'warn'. (opensearch-project#262)

Signed-off-by: dblock <dblock@amazon.com>

Removed default values from param description (opensearch-project#264)

Signed-off-by: saimedhi <saimedhi@amazon.com>

Generate _opendistro endpoints through merger tool (opensearch-project#257)

* Generate _opendistro endpoints through merger tool

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

* # Renamed `replaced` with `superseded`

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

* # Rebased DEVELOPER_GUIDE.md

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

* # Set Tabsize from 4 to 2

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

---------

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

Add _plugins/_notifications/channels API spec (opensearch-project#256)

* Add _plugins/_notifications/channels API

Signed-off-by: Sokratis Papadopoulos <sokratis.papadopoulos@cern.ch>

Fix obvious lints. (opensearch-project#265)

* Fix cosmetic autoformat lints.
* Fixed @typescript-eslint/no-unused-vars.
* Fixed eqeqeq.
* Fixed @typescript-eslint/consistent-type-imports.
* Fixed no-useless-return.
* Fixed @typescript-eslint/array-type.
* Rebased with changes on main.

Signed-off-by: dblock <dblock@amazon.com>

Update list notification channels url for externalDocs (opensearch-project#267)

Signed-off-by: Sokratis Papadopoulos <sokratis.papadopoulos@cern.ch>
Co-authored-by: Sokratis Papadopoulos <sokratis.papadopoulos@cern.ch>

Updated/Corrected Docs (opensearch-project#270)

* Updated/Corrected Docs

- README.md
- CLIENT_GENERATOR_GUIDE.md
- DEVELOPER_GUIDE.md
- ./tools/README.md

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

* # minor corrections

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

* # minor corrections

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

---------

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

Add lychee github action  for links checking (opensearch-project#269)

Corrected content type for bulk operations (opensearch-project#275)

* Corrected content type for bulk operations

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

* # linting

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

---------

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

Validate _superseded_operations.yaml against its JSON schema (opensearch-project#276)

* Validate _superseded_operations.yaml against its JSON schema

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

* # lint

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

* # lint

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

---------

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

Fixed Linting for Tools (opensearch-project#278)

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

Removed Root file since it's not needed anymore (opensearch-project#279)

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

Fixed missing global params (opensearch-project#280)

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

Added validation for _info.yaml (opensearch-project#281)

DRY'ed JSON schema validation logic

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

Implement inline object schema validator (opensearch-project#282)

* Implement inline object schema validator and underlying visitor pattern

Signed-off-by: Thomas Farr <tsfarr@amazon.com>

* Fix spec lint error

Signed-off-by: Thomas Farr <tsfarr@amazon.com>

---------

Signed-off-by: Thomas Farr <tsfarr@amazon.com>

oops

fix lint
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.

2 participants