[Reporting] Use updated saved object resolver #113641

dokmic · 2021-10-01T16:00:12Z

Summary

Checklist

Unit or functional tests were updated or added to match the most common scenarios

ppisljar · 2021-10-04T14:37:04Z

x-pack/plugins/reporting/server/export_types/csv/create_job.ts

@@ -21,10 +21,12 @@ export const createJobFnFactory: CreateJobFnFactory<
    );

    const savedObjectsClient = context.core.savedObjects.client;
-    const indexPatternSavedObject = (await savedObjectsClient.get(
+    const {
+      saved_object: { attributes: indexPatternSavedObject },


you should check if you got the object

I've checked the saved objects client implementation, and it matches the types they claim (server and public). The saved_object property is always an object.

what happens if you request an object which doesnt exist, or if you get into conflict scenario? https://www.elastic.co/guide/en/kibana/master/sharing-saved-objects.html#sharing-saved-objects-step-3

In that case, the save_object is still there. You can see that in the code (server and public).

ppisljar · 2021-10-05T05:41:51Z

x-pack/plugins/reporting/server/export_types/csv/create_job.ts

@@ -21,10 +21,12 @@ export const createJobFnFactory: CreateJobFnFactory<
    );

    const savedObjectsClient = context.core.savedObjects.client;


actually, we shouldnt be using saved objects client here. we should use data.indexPatterns service. can you please update that ?

The problem is that the context.core is a setup contract that doesn't have the data plugin instance. I can work that around, but this is a deprecated job type that should be removed in 8.0. And the issue is labeled as v8.0.0 only. Fixing that here will produce extra work with removing that later without any impact on the final result.

ok if we are removing this in 8.0 then its ok. (cc @tsullivan are we sure its gonna get removed in 8.0?)

the problem with this approach is that you need to know internals of data views (index patterns), like how are they stored etc. Any change on index patterns will break your logic and its hard to detect that. Saved objects should be loaded only by their owners and nobody else.

export_types/csv is not being removed in 8.0. The code isn't accessible through the UI, but we know from telemetry data that users are still generating these exports using automation that was set up in an older version. So we don't want their automation to immediately break once they upgrade. Once the removal won't affect many users we will do it.

Alright, then I guess we should use the data plugin here instead. And correct the warning saying that it will be removed in 8.0 😄

@ppisljar @tsullivan I've updated the factory to reuse the data plugin. Could you please take another look?

ppisljar · 2021-10-18T04:48:04Z

x-pack/plugins/reporting/server/export_types/csv/create_job.ts


    return {
      isDeprecated: true,
-      indexPatternSavedObject,
+      indexPatternSavedObject: omitBy(


why not just pass dataView in ?

I am afraid that's not going to work. We put it here to serialize and deserialize that later.

you should only store data view id then, and load it again later using data view apis

It makes total sense, thanks. I've fixed that.

ppisljar · 2021-10-18T04:55:20Z

x-pack/plugins/reporting/server/export_types/csv/generate_csv/field_format_map.ts

-    const fieldFormatMap = JSON.parse(indexPatternSavedObject.attributes.fieldFormatMap);
-    Object.keys(fieldFormatMap).forEach((fieldName) => {
-      const formatConfig: FieldFormatConfig = fieldFormatMap[fieldName];
+  if (_.has(indexPatternSavedObject, 'fieldFormatMap')) {


this whole function is pretty much constructing what's already available on data views.
if you don't convert dataview to your indexPatternSavedObject (which you shouldn't) then you can just use
dataView.fieldFormatMap or even better use provided functions on the dataview:
dataview.getFormatterForField(field)

That comes from the ES document as a plain object. In the job factory, I tried to gather all the data we used previously and store that in a similar way.

dokmic · 2021-10-18T08:10:48Z

@elasticmachine merge upstream

…eprecated CSV export type

dokmic · 2021-10-20T11:25:59Z

@elasticmachine merge upstream

kibanamachine · 2021-10-20T13:46:03Z

💚 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
`reporting`	134	136	+2

Unknown metric groups

API count

id	before	after	diff
`reporting`	135	137	+2

History

💛 Build #163121 was flaky 07cee74
💔 Build #162921 failed 8e2d47ec4516287930a48e01be8d9d2f7c398e32
💚 Build #161896 succeeded 2b925c292c774e413da711f213bc1822ae38601c
💚 Build #161408 succeeded 1829cf2d810f9c47a0890a9341c28bb09c9ee17a
💔 Build #161375 failed 0e3dfd1d656ab93238353b6720cc7258a0d06f90

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

jportner · 2021-10-20T14:41:53Z

I just wanted to check on the status of this, it's blocking #114408

ppisljar

code LGTM

tsullivan

LGTM

* Update deprecated PDF job factory to use updated saved object resolver * Reuse data plugin service to gather index pattern attributes in the deprecated CSV export type

kibanamachine · 2021-10-25T14:23:22Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 113641 or prevent reminders by adding the backport:skip label.

kibanamachine · 2021-10-26T14:33:23Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 113641 or prevent reminders by adding the backport:skip label.

dokmic added review (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:AppServicesUx labels Oct 1, 2021

dokmic force-pushed the feature/112565 branch 2 times, most recently from 3c8b2dc to 0763c9f Compare October 4, 2021 09:31

dokmic marked this pull request as ready for review October 4, 2021 11:43

dokmic requested review from a team as code owners October 4, 2021 11:43

ppisljar reviewed Oct 4, 2021

View reviewed changes

ppisljar reviewed Oct 5, 2021

View reviewed changes

mattkime mentioned this pull request Oct 13, 2021

[data views] Make data view saved objects share capable #114408

Merged

dokmic force-pushed the feature/112565 branch 3 times, most recently from 0e3dfd1 to 1829cf2 Compare October 15, 2021 13:07

ppisljar reviewed Oct 18, 2021

View reviewed changes

dokmic force-pushed the feature/112565 branch from 2b925c2 to 8e2d47e Compare October 19, 2021 19:29

dokmic added 4 commits October 20, 2021 09:36

Update deprecated CSV generator to use updated saved object resolver

cadba58

Update deprecated PDF job factory to use updated saved object resolver

98c51e2

Reuse data plugin service to gather index pattern attributes in the d…

40169db

…eprecated CSV export type

Move data view gathering to the CSV job executor

07cee74

dokmic force-pushed the feature/112565 branch from 8e2d47e to 07cee74 Compare October 20, 2021 07:36

Merge branch 'master' into feature/112565

7906435

ppisljar approved these changes Oct 21, 2021

View reviewed changes

tsullivan approved these changes Oct 21, 2021

View reviewed changes

dokmic merged commit 6b64103 into elastic:master Oct 21, 2021

dokmic deleted the feature/112565 branch October 21, 2021 14:14

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 25, 2021

jportner added the backport:skip This commit does not require backporting label Oct 26, 2021

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Reporting] Use updated saved object resolver #113641

[Reporting] Use updated saved object resolver #113641

dokmic commented Oct 1, 2021

ppisljar Oct 4, 2021

dokmic Oct 6, 2021

ppisljar Oct 7, 2021

dokmic Oct 7, 2021

ppisljar Oct 5, 2021

dokmic Oct 6, 2021

ppisljar Oct 7, 2021

tsullivan Oct 7, 2021 •

edited

Loading

dokmic Oct 7, 2021

dokmic Oct 13, 2021

ppisljar Oct 18, 2021

dokmic Oct 18, 2021

ppisljar Oct 18, 2021

dokmic Oct 20, 2021

ppisljar Oct 18, 2021

dokmic Oct 18, 2021

dokmic commented Oct 18, 2021

dokmic commented Oct 20, 2021

kibanamachine commented Oct 20, 2021

API count

jportner commented Oct 20, 2021

ppisljar left a comment

tsullivan left a comment

kibanamachine commented Oct 25, 2021

kibanamachine commented Oct 26, 2021

		@@ -21,10 +21,12 @@ export const createJobFnFactory: CreateJobFnFactory<
		);

		const savedObjectsClient = context.core.savedObjects.client;

[Reporting] Use updated saved object resolver #113641

[Reporting] Use updated saved object resolver #113641

Conversation

dokmic commented Oct 1, 2021

Summary

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsullivan Oct 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dokmic commented Oct 18, 2021

dokmic commented Oct 20, 2021

kibanamachine commented Oct 20, 2021

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

API count

History

jportner commented Oct 20, 2021

ppisljar left a comment

Choose a reason for hiding this comment

tsullivan left a comment

Choose a reason for hiding this comment

kibanamachine commented Oct 25, 2021

kibanamachine commented Oct 26, 2021

tsullivan Oct 7, 2021 •

edited

Loading