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

[CM] Soften response validation #166919

Merged
merged 15 commits into from
Sep 28, 2023

Conversation

Dosant
Copy link
Contributor

@Dosant Dosant commented Sep 21, 2023

Summary

Close #167152

Log a warning instead of throwing an error in saved_object_content_storage when response validation failed.

We decided to do this as a precaution and as a follow up to an issue found in saved search #166886 where storage started failing because of too strict validation.

As of this PR the saved_object_content_storage covers and this change cover:

  • search
  • index_pattern
  • dashboard
  • lens
  • maps

For other types we agreed with @dej611 that instead of applying the same change for other types (visualization, graph, annotation) the team would look into migrating their types to also use saved_object_content_storage #167421

@Dosant Dosant changed the title wip [CM] Soften response validation Sep 21, 2023
@Dosant Dosant added release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) Feature:Content Management User generated content (saved objects) management labels Sep 27, 2023
@Dosant Dosant marked this pull request as ready for review September 27, 2023 15:19
@Dosant Dosant requested review from a team as code owners September 27, 2023 15:19
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@Dosant Dosant requested a review from dej611 September 27, 2023 15:19
Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kibana-presentation and kibana-gis changes LGTM
code review only

Copy link
Contributor

@drewdaemon drewdaemon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, code review only.

Feels like a good case to validate with API integration tests (not saying that needs to be done here).

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pulled and tested locally in Discover, and it works as expected with throwOnResultValidationError on and off. Thanks for addressing this and adding the tests! LGTM 👍

Copy link
Contributor

@sebelga sebelga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Dosant
Copy link
Contributor Author

Dosant commented Sep 28, 2023

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/content-management-utils 124 126 +2
Unknown metric groups

API count

id before after diff
@kbn/content-management-utils 189 191 +2

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Content Management User generated content (saved objects) management release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CM] Soften response validation
8 participants