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

Made the InlineGet source field optional. #351

Closed
wants to merge 1 commit into from

Conversation

BrendonFaleiro
Copy link

Address #350

Signed-off-by: Brendon Faleiro [email protected]

Copy link

Changes Analysis

Commit SHA: 3bb379e
Comparing To SHA: 33aa91a

API Changes

Summary

└─┬Components
  ├─┬_common:InlineGet
  │ └──[➖] required (26493:11)❌ 
  └─┬_common:InlineGetDictUserDefined
    └──[➖] required (26515:11)❌ 

Document Element Total Changes Breaking Changes
components 2 2
  • BREAKING Changes: 2 out of 2
  • Removals: 2
  • Breaking Removals: 2

Report

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

API Coverage

Before After Δ
Covered (%) 483 (47.31 %) 483 (47.31 %) 0 (0 %)
Uncovered (%) 538 (52.69 %) 538 (52.69 %) 0 (0 %)
Unknown 24 24 0

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.

Thanks. Do you have time to write a test for this change? We're trying to use regressions to improve coverage.

Something minimal that calls this API, it should fail on the schema check as long as it doesn't return _source, https://github.com/opensearch-project/opensearch-api-specification/tree/main/tests

This is part of a bulk API if I am not mistaken, I'm adding support for that in #355, you can add a test on top of that.

@dblock
Copy link
Member

dblock commented Jul 3, 2024

I added a test on top of this change in #375.

@dblock dblock closed this Jul 3, 2024
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