[TSVB] Remove request facade and type server code#92964
[TSVB] Remove request facade and type server code#92964sulemanof merged 17 commits intoelastic:masterfrom
Conversation
…ade_removal # Conflicts: # src/plugins/vis_type_timeseries/server/lib/get_fields.ts # src/plugins/vis_type_timeseries/server/lib/get_vis_data.ts # src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts # src/plugins/vis_type_timeseries/server/lib/vis_data/annotations/get_request_params.js # src/plugins/vis_type_timeseries/server/lib/vis_data/series/get_request_params.js
| fakeRequest: FakeRequest, | ||
| options: GetVisDataOptions | ||
| ) => ReturnType<GetVisData>; | ||
| // ideally this should be VisPayload type, but currently has inconsistencies with x-pack/plugins/infra/server/lib/adapters/framework/kibana_framework_adapter.ts |
There was a problem hiding this comment.
looks like @dziyanadzeraviankina already fixed that issue here #91977
There was a problem hiding this comment.
I would not like to make any changes in infra code in this PR, let's ask @dziyanadzeraviankina to remove this any and comment when resolving merge conflicts?
|
|
||
| public get searchTimezone() { | ||
| return getTimezoneFromRequest(this.request); | ||
| return getTimezoneFromRequest(this.request as VisTypeTimeseriesVisDataRequest); |
There was a problem hiding this comment.
are you sure we need that casting here? in line 24 you already set VisTypeTimeseriesRequest type for request. getTimezoneFromRequest also works with VisTypeTimeseriesVisDataRequest
There was a problem hiding this comment.
VisTypeTimeseriesRequest is a union type of VisTypeTimeseriesFieldsRequest | VisTypeTimeseriesVisDataRequest,
but getTimezoneFromRequest expects only VisTypeTimeseriesVisDataRequest.
If you don't like type casting here, I made a type guard in the recent commit (but it looks overloaded IMHO)
src/plugins/vis_type_timeseries/server/lib/vis_data/series/handle_response_body.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Thank you for that cleanup. No red flags from my side. @stratoula @VladLasitsa please review.
| * @returns {Object} doc - processed body | ||
| */ | ||
| export async function buildAnnotationRequest(...args) { | ||
| export async function buildAnnotationRequest(...args: any[]) { |
There was a problem hiding this comment.
Can we type it? Or at least replace it with unknown?
src/plugins/vis_type_timeseries/server/lib/vis_data/annotations/get_request_params.ts
Outdated
Show resolved
Hide resolved
src/plugins/vis_type_timeseries/server/lib/vis_data/series/get_request_params.ts
Outdated
Show resolved
Hide resolved
…ade_removal # Conflicts: # api_docs/core.json # api_docs/vis_type_timeseries.json
stratoula
left a comment
There was a problem hiding this comment.
LGTM, thanx @sulemanof ❤️
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
|
@elasticmachine run elasticsearch-ci/docs |
* Remove request facade and update search strategies * Use typescript * Type files * Update structure * Update tests * Type annotations * Fix type for infra * Enhance types * Remove generics * Use constant * Update docs * Type capabilities
* Remove request facade and update search strategies * Use typescript * Type files * Update structure * Update tests * Type annotations * Fix type for infra * Enhance types * Remove generics * Use constant * Update docs * Type capabilities
Summary
Part of #57342
This removes
ReqFacadetype to provide a nicer extension point for search strategies.Next files were converted to TypeScript to avoid possible errors with request facade removal:
annotations/build_request_body.tsannotations/get_request_params.tsvis_data/get_annotations.tsvis_data/get_series_data.tsvis_data/get_table_data.tshelpers/get_es_query_uisettings.tsvis_data/series/get_request_params.tsvis_data/series/handle_response_body.tsChecklist
Delete any items that are not applicable to this PR.
For maintainers