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

[Search Sessions] fix updating deleting sessions from non-default space #96123

Conversation

Dosant
Copy link
Contributor

@Dosant Dosant commented Apr 2, 2021

Summary

Potential bare minimum fix for #96124
The problem was that updates/deletes of sessions from non-default spaces failed with 404 because we were trying to update/delete them in the default space.
I also improved logging a bit.

While debugging we identify other potential improvements, but they out of scope for this patch pr: #96131

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@Dosant Dosant mentioned this pull request Apr 2, 2021
11 tasks
@Dosant Dosant added release_note:fix Team:AppServices v7.12.1 v7.13.0 v8.0.0 Feature:Search Querying infrastructure in Kibana bug Fixes for quality problems that affect the customer experience labels Apr 2, 2021
@Dosant Dosant marked this pull request as ready for review April 2, 2021 15:26
@Dosant Dosant requested a review from a team as a code owner April 2, 2021 15:26
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

@Dosant Dosant requested review from lukasolson, lizozom and a team April 2, 2021 15:26
Copy link
Member

@legrego legrego left a comment

Choose a reason for hiding this comment

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

Changes to the logic LGTM

Can we update the unit tests in x-pack/plugins/data_enhanced/server/search/session/check_running_sessions.test.ts to verify that we're calling the Saved Objects Client with the appropriate namespace for the update and delete operations?

Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

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

LGTM

@lizozom
Copy link
Contributor

lizozom commented Apr 4, 2021

@elasticmachine merge upstream

@lizozom lizozom requested a review from legrego April 5, 2021 08:13
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@lizozom lizozom added the auto-backport Deprecated: Automatically backport this PR after it's merged label Apr 5, 2021
@lizozom lizozom merged commit 8c8323a into elastic:master Apr 5, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Apr 5, 2021
…ce (elastic#96123)

* add spaces test

* fix updating and deleting sessions in non-default space

* revert back to batch update

* Add space tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Liza K <liza.katz@elastic.co>
@kibanamachine
Copy link
Contributor

💔 Backport failed

❌ 7.12: Commit could not be cherrypicked due to conflicts
7.x / #96210

Successful backport PRs will be merged automatically after passing CI.

To backport manually run:
node scripts/backport --pr 96123

lizozom pushed a commit to lizozom/kibana that referenced this pull request Apr 5, 2021
…ce (elastic#96123)

* add spaces test

* fix updating and deleting sessions in non-default space

* revert back to batch update

* Add space tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Liza K <liza.katz@elastic.co>
# Conflicts:
#	x-pack/plugins/data_enhanced/server/search/session/check_running_sessions.ts
kibanamachine added a commit that referenced this pull request Apr 5, 2021
…ce (#96123) (#96210)

* add spaces test

* fix updating and deleting sessions in non-default space

* revert back to batch update

* Add space tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Liza K <liza.katz@elastic.co>

Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
Co-authored-by: Liza K <liza.katz@elastic.co>
lizozom added a commit that referenced this pull request Apr 5, 2021
…ce (#96123) (#96211)

* add spaces test

* fix updating and deleting sessions in non-default space

* revert back to batch update

* Add space tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Liza K <liza.katz@elastic.co>
# Conflicts:
#	x-pack/plugins/data_enhanced/server/search/session/check_running_sessions.ts

Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
Dosant added a commit that referenced this pull request May 12, 2021
Co-authored-by: Kaarina Tungseth <kaarina.tungseth@elastic.co>
Dosant added a commit to Dosant/kibana that referenced this pull request May 12, 2021
…lastic#99617)

Co-authored-by: Kaarina Tungseth <kaarina.tungseth@elastic.co>
Dosant added a commit that referenced this pull request May 12, 2021
…#99965)

Co-authored-by: Kaarina Tungseth <kaarina.tungseth@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated: Automatically backport this PR after it's merged bug Fixes for quality problems that affect the customer experience Feature:Search Querying infrastructure in Kibana release_note:fix v7.12.1 v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants