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

[data views] Fix overwrite param for create #160953

Merged
merged 12 commits into from
Jul 4, 2023

Conversation

mattkime
Copy link
Contributor

@mattkime mattkime commented Jun 30, 2023

Summary

Under some circumstances passing override to POST /api/data_views/data_view would fail. Its now fixed.

To test - Try using the override param from the Kibana dev console. I found it reproduced the problem before the fix and shows its resolved after the fix. The problem did not appear in the integration tests.

I suspect the problem had to do with how quickly the delete was performed - if it completed before the create command then everything was fine. If it didn't then the error would appear. Passing the overwrite param to the saved object client eliminates the possibility of the delete failing to complete.

Closes #161016

@mattkime mattkime changed the title pass overwrite to saved object client [data views] Fix overwrite param for create Jun 30, 2023
@mattkime mattkime self-assigned this Jun 30, 2023
@mattkime mattkime added backport:all-open Backport to all branches that could still receive a release Feature:Data Views Data Views code and UI - index patterns before 8.0 Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. release_note:fix labels Jun 30, 2023
@mattkime mattkime marked this pull request as ready for review July 1, 2023 15:41
@mattkime mattkime requested a review from a team as a code owner July 1, 2023 15:41
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@@ -1000,6 +1000,7 @@ export class DataViewsService {
const response: SavedObject<DataViewAttributes> = (await this.savedObjectsClient.create(body, {
id: dataView.id,
initialNamespaces: dataView.namespaces.length > 0 ? dataView.namespaces : undefined,
overwrite: override,
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to consistently use overwrite or override?

@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
data 2579 2583 +4
dataViews 258 268 +10
total +14

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
dataViews 44.6KB 44.7KB +12.0B
Unknown metric groups

API count

id before after diff
dataViews 1048 1051 +3

ESLint disabled line counts

id before after diff
enterpriseSearch 14 16 +2
securitySolution 411 415 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 15 17 +2
securitySolution 490 494 +4
total +6

History

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

cc @mattkime

Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@mattkime mattkime merged commit 803d139 into elastic:main Jul 4, 2023
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 4, 2023
## Summary

Under some circumstances passing `override` to `POST
/api/data_views/data_view` would fail. Its now fixed.

To test - Try using the override param from the Kibana dev console. I
found it reproduced the problem before the fix and shows its resolved
after the fix. The problem did not appear in the integration tests.

I suspect the problem had to do with how quickly the delete was
performed - if it completed before the create command then everything
was fine. If it didn't then the error would appear. Passing the
overwrite param to the saved object client eliminates the possibility of
the delete failing to complete.

Closes elastic#161016

(cherry picked from commit 803d139)
@kibanamachine
Copy link
Contributor

💔 Some backports could not be created

Status Branch Result
7.17 Backport failed because of merge conflicts
8.9

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 160953

Questions ?

Please refer to the Backport tool documentation

jloleysens added a commit to jloleysens/kibana that referenced this pull request Jul 5, 2023
* main: (354 commits)
  [Synthetics] Overview page fix last refresh value display (elastic#161086)
  [Synthetics] Remove TLS alert option for ICMP monitor (elastic#161173)
  fixing the path of manifets for hints autodiscover (elastic#161075)
  [Fleet] Fix permissions in integrations Assets page (elastic#161233)
  Update publicBaseUrl warning id (elastic#161204)
  [ML] Fix Anomaly Explorer URL for alerting context with non-default space  (elastic#160899)
  [Enterprise Search]Add 404 error handling for mappings and documents endpoints (elastic#161203)
  [Logs Shared] Move LogStream and LogView into new shared plugin (elastic#161151)
  [Security Solutions] Fix  CellActions component should hide ShowTopN action for nested fields (elastic#159645)
  [SecuritySolutions] Remove filter actions from Cases alerts table and fix show_top_n action (elastic#161150)
  [Infrastructure UI] Add strict payload validation to inventory_views endpoint (elastic#160852)
  [api-docs] 2023-07-05 Daily api_docs build (elastic#161225)
  Fix errors in custom metric payload in SLO dev docs (elastic#161141)
  [data views] Fix overwrite param for create (elastic#160953)
  [Synthetics] Perform params API HTTP migration (elastic#160575)
  [Cloud Security][FTR]Refactor API FTR to use .to.eql instead of .to.be  (elastic#160694)
  Have SLO routes return a 403 instead of a 400 when user has an insufficient license (elastic#161193)
  [Discover] Fix shared links flaky test (elastic#161172)
  [ftr] Improve FTR error handling for NoSuchSessionError (elastic#161025)
  skip flaky suite (elastic#151981)
  ...
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jul 5, 2023
* main: (354 commits)
  [Synthetics] Overview page fix last refresh value display (elastic#161086)
  [Synthetics] Remove TLS alert option for ICMP monitor (elastic#161173)
  fixing the path of manifets for hints autodiscover (elastic#161075)
  [Fleet] Fix permissions in integrations Assets page (elastic#161233)
  Update publicBaseUrl warning id (elastic#161204)
  [ML] Fix Anomaly Explorer URL for alerting context with non-default space  (elastic#160899)
  [Enterprise Search]Add 404 error handling for mappings and documents endpoints (elastic#161203)
  [Logs Shared] Move LogStream and LogView into new shared plugin (elastic#161151)
  [Security Solutions] Fix  CellActions component should hide ShowTopN action for nested fields (elastic#159645)
  [SecuritySolutions] Remove filter actions from Cases alerts table and fix show_top_n action (elastic#161150)
  [Infrastructure UI] Add strict payload validation to inventory_views endpoint (elastic#160852)
  [api-docs] 2023-07-05 Daily api_docs build (elastic#161225)
  Fix errors in custom metric payload in SLO dev docs (elastic#161141)
  [data views] Fix overwrite param for create (elastic#160953)
  [Synthetics] Perform params API HTTP migration (elastic#160575)
  [Cloud Security][FTR]Refactor API FTR to use .to.eql instead of .to.be  (elastic#160694)
  Have SLO routes return a 403 instead of a 400 when user has an insufficient license (elastic#161193)
  [Discover] Fix shared links flaky test (elastic#161172)
  [ftr] Improve FTR error handling for NoSuchSessionError (elastic#161025)
  skip flaky suite (elastic#151981)
  ...
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label May 6, 2024
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 160953 locally

@mattkime mattkime added the backport:skip This commit does not require backporting label Aug 15, 2024
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 15, 2024
@mattkime mattkime removed the backport:all-open Backport to all branches that could still receive a release label Aug 15, 2024
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:Data Views Data Views code and UI - index patterns before 8.0 release_note:fix Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

POST data_view with overwrite recieves 400 error
6 participants