-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Reporting] Add warning logs about CSV export type being deprecated #104025
Changes from 2 commits
5f57901
73ea0ad
62eb82c
41b4aea
68133c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,9 +5,9 @@ | |
* 2.0. | ||
*/ | ||
|
||
import { UnwrapPromise } from '@kbn/utility-types'; | ||
import { i18n } from '@kbn/i18n'; | ||
import { ResponseError } from '@elastic/elasticsearch/lib/errors'; | ||
import { i18n } from '@kbn/i18n'; | ||
import { UnwrapPromise } from '@kbn/utility-types'; | ||
import { ElasticsearchClient } from 'src/core/server'; | ||
import { ReportingCore } from '../../'; | ||
import { ReportDocument } from '../../lib/store'; | ||
|
@@ -87,6 +87,7 @@ export function jobsQueryFactory(reportingCore: ReportingCore) { | |
elasticsearchClient.search({ body, index: getIndex() }) | ||
); | ||
|
||
// FIXME: return the info in ReportApiJSON format; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One thing became clear from updating the API functional tests, the data formats of the Reporting endpoints are inconsistent.
To make it consistent, all the APIs should return The comments are added to this file, which is responsible for handling API requests that query jobs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of the FIXMEs are being handled in the next PR: #102833 |
||
return response?.body.hits?.hits ?? []; | ||
}, | ||
|
||
|
@@ -139,6 +140,7 @@ export function jobsQueryFactory(reportingCore: ReportingCore) { | |
return; | ||
} | ||
|
||
// FIXME: return the info in ReportApiJSON format; | ||
return response.body.hits.hits[0] as ReportDocument; | ||
}, | ||
|
||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: perhaps we could rephrase this as:
My concern with the current version is that mentioning an endpoint
/generate/csv
is a bit too implementation-level detail. WDYT?Additionally, I think we should wrap this in
i18n.translate
.If we can link them to somewhere in the docs that would be A+, but this can definitely be addressed in a follow up PR if you agree!