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 / search session] BWCA all the routes #158981

Merged
merged 20 commits into from
Jun 16, 2023

Conversation

mattkime
Copy link
Contributor

@mattkime mattkime commented Jun 3, 2023

Summary

All routes

  • Use versioned router
  • Validate responses
  • All responses are typed with response types, separate from internal api types. This is to help prevent unacknowledged changes to the api.

Closes #158944

@mattkime mattkime self-assigned this Jun 3, 2023
@mattkime mattkime added Feature:Search Sessions Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. release_note:skip Skip the PR/issue when compiling release notes labels Jun 3, 2023
@mattkime mattkime changed the title add versioning to search session routes [data / search session] BWCA all the routes Jun 3, 2023
@mattkime
Copy link
Contributor Author

So CI doesn't overwrite, these are failing -

[job] [logs] Jest Tests #9 / registerSessionRoutes cancel calls cancelSession with id
[job] [logs] Jest Tests #9 / registerSessionRoutes delete calls deleteSession with id
[job] [logs] Jest Tests #9 / registerSessionRoutes extend calls extendSession with id
[job] [logs] Jest Tests #9 / registerSessionRoutes find calls findSession with options
[job] [logs] Jest Tests #9 / registerSessionRoutes status calls getSessionStatus with sessionId
[job] [logs] Jest Tests #9 / registerSessionRoutes update calls updateSession with id and attributes

@mattkime mattkime marked this pull request as ready for review June 13, 2023 15:19
@mattkime mattkime requested review from a team as code owners June 13, 2023 15:19
@elasticmachine
Copy link
Contributor

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

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.

Code changes look good, and I tested search session functionality locally in Discover and the management screen, and all endpoints seem to be working as expected. LGTM

},
},
async (context, request, res) => {
const { id } = request.params;
const { expires } = request.body;
try {
const searchContext = await context.search;
const response = await searchContext.extendSession(id, new Date(expires));
await searchContext.extendSession(id, new Date(expires));
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious why the return type here changed. Were we just not using it?

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

LGTM!

@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 2578 2579 +1

Page load bundle

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

id before after diff
data 404.7KB 404.7KB +89.0B
Unknown metric groups

API count

id before after diff
data 3302 3303 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 13 15 +2
securitySolution 410 414 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 14 16 +2
securitySolution 493 497 +4
total +6

History

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

cc @mattkime

@mattkime mattkime merged commit 5c5add2 into elastic:main Jun 16, 2023
@kibanamachine kibanamachine added v8.9.0 backport:skip This commit does not require backporting labels Jun 16, 2023
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:Search Sessions release_note:skip Skip the PR/issue when compiling release notes Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[search-session] Routes must satisfy bwca
7 participants