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

Fix integration child mapping validation bug #1234

Merged
merged 3 commits into from
Nov 22, 2023

Conversation

Swiddis
Copy link
Collaborator

@Swiddis Swiddis commented Nov 9, 2023

Description

As described in #1148, integrations fails to validate some kinds of schemas, where child mappings don't align with the name of the mappings' properties. This is true for some SS4O mappings (like http and communication), but not all of them, which led to the issue being undetected.

This PR fixes the bug and adds a corresponding test case. In the future, we should look at adding an integration test that tries to connect an integration to its own sample data, which would have caught this bug significantly faster.

Issues Resolved

Fixes validation bug for #1148

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

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.

@Swiddis
Copy link
Collaborator Author

Swiddis commented Nov 9, 2023

Test failures seem to be entirely unrelated -- has something changed upstream?

Signed-off-by: Simeon Widdis <[email protected]>
This reverts commit b2c533f.

Signed-off-by: Simeon Widdis <[email protected]>
@Swiddis
Copy link
Collaborator Author

Swiddis commented Nov 13, 2023

Closing as we're not backporting to 2.9

@Swiddis Swiddis closed this Nov 13, 2023
@Swiddis
Copy link
Collaborator Author

Swiddis commented Nov 13, 2023

Reopening -- On discussion with team, it's important enough to backport for 2.9

@Swiddis Swiddis reopened this Nov 13, 2023
@Swiddis Swiddis added the integrations Used to denote items related to the Integrations project label Nov 14, 2023
@ps48
Copy link
Member

ps48 commented Nov 22, 2023

@Swiddis you might need to fix tests here before merging

@Swiddis
Copy link
Collaborator Author

Swiddis commented Nov 22, 2023

It looks like all the failing tests aren't from this PR, I was asking around and got told that the tests on this branch are unstable. I searched for any failures related to these changes in the logs and didn't find anything.

@Swiddis Swiddis merged commit 46259f2 into opensearch-project:2.9 Nov 22, 2023
7 of 13 checks passed
@Swiddis Swiddis deleted the fix-validation branch November 22, 2023 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working integrations Used to denote items related to the Integrations project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants