[Reporting] Use a shim for server config #62086

tsullivan · 2020-03-31T22:37:14Z

Summary

This PR is to break out some changes from #61696. It retains the changes that update the call signatures of the code, but this PR does not actually change the source of config: the config data still comes from server.config().get() -- not the New Platform yet.

Checklist

Delete any items that are not applicable to this PR.

~~[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~
~~[ ] Documentation was added for features that require explanation or tutorials~~
Unit or functional tests were updated or added to match the most common scenarios
~~[ ] This was checked for keyboard-only and screenreader accessibility~~
~~[ ] This renders correctly on smaller devices using a responsive layout. (You can test this in your browser~~
~~[ ] This was checked for cross-browser compatibility, including a check against IE11~~

For maintainers

This was checked for breaking API changes and was labeled appropriately

x-pack/legacy/plugins/reporting/server/legacy.ts

tsullivan · 2020-04-02T19:18:41Z

x-pack/legacy/plugins/reporting/server/routes/generate_from_savedobject_immediate.ts

@@ -51,15 +51,8 @@ export function registerGenerateCsvFromSavedObjectImmediate(
      const request = makeRequestFacade(legacyRequest);
      const logger = parentLogger.clone(['savedobject-csv']);
      const jobParams = getJobParamsFromRequest(request, { isImmediate: true });
-
-      /* TODO these functions should be made available in the export types registry:


I think if we make the "execute job function factory" functions return immediately instead of returning a promise, I'd be satisfied this TODO is no longer needed.

BTW a few more of these TODOs have been removed, above

tsullivan · 2020-04-02T19:23:34Z

x-pack/legacy/plugins/reporting/server/lib/create_worker.ts

+        JobParamsType,
+        unknown,
+        unknown,
+        ImmediateExecuteFn<JobParamsType> | ESQueueWorkerExecuteFn<JobDocPayloadType>


replacing the any here helps provide the correct typing for jobExecutor

tsullivan · 2020-04-02T19:30:19Z

x-pack/legacy/plugins/reporting/export_types/printable_pdf/server/execute_job/index.ts

  parentLogger: Logger
 ) {
-  const browserDriverFactory = await reporting.getBrowserDriverFactory();


This PR starts moving async things in execute job factory function to be resolved in the function that is returned instead of before, so the factory function can return immediately instead of returning a promise. Having reporting.getConfig() return synchronously (instead of async) kind of helps get that idea working.

tsullivan · 2020-04-03T17:42:03Z

x-pack/legacy/plugins/reporting/server/types.d.ts

@@ -16,7 +16,7 @@ import { ReportingConfig, ReportingConfigType } from './core';
 export interface ReportingSetupDeps {
  elasticsearch: ElasticsearchServiceSetup;
  security: SecurityPluginSetup;
-  usageCollection: UsageCollectionSetup;
+  usageCollection?: UsageCollectionSetup;


cc @ @afharo - this commit is from your feedback on #61696

kibanamachine · 2020-04-03T19:24:13Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 67acc88

History

💚 Build #38334 succeeded 8d95558
💚 Build #38289 succeeded a9f7f60
💚 Build #38269 succeeded 1561725
💚 Build #38234 succeeded 1705882
💚 Build #38046 succeeded 79b1481fa7fcbc4fc84f1b0538904e7f5c63a5d3

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

joelgriffith

whew, LGTM, the difference in additions/subtractions

* config shim * simplify route register calls * switch to in-sync worker functions * fix tests * comment * fix set up config with defaults * reduce loc change * remove test for removed file * reportingconfigtype * revert changing executeJobFactory to synchronous * imports cleanup * Clean up some awaits * undo comment * clean up async * clean up imports * add warning logs for config defaults * Move around some config shim code * Register routes params take ReportingCore * usageCollection is an optional dependency

…into event-log/query-support * 'event-log/query-support' of github.com:gmmorris/kibana: (41 commits) [jenkins] refer to sizes in most pipeline code (elastic#62082) skip flaky suite (elastic#60470) [Discover] Fix flaky FT in field visualize (elastic#62418) [ML] Data Frame Analytics: Fix feature importance (elastic#61761) [Reporting] Use a shim for server config (elastic#62086) [Reporting] Fix reporting for non-default spaces (elastic#62226) Fix bug that coerced empty scaled float value to 0 (elastic#62251) [SIEM] [Detection Engine] Remove has manage api keys requireme… (elastic#62446) [Maps] Safely handle empty string and invalid strings from EuiColorPicker (elastic#62507) Reporting/bug more blacklisted headers (elastic#62389) [SIEM] Prevent undefined behavior in our ML popover (elastic#62498) [SIEM] [Detection Engine] remove all unknowns from all rules t… (elastic#62327) base changes for active/current node styling (elastic#62007) [kbn/ui-shared-deps] expand and split (elastic#62364) [ML] DF Analytics - ensure destination index pattern created (elastic#62450) Mark rule run as failure if there was an error (elastic#62383) Add docs for metric explorer alerts (elastic#62314) skip flaky suite (elastic#62281) [SIEM][Detection Engine] Fixes export of single rule and the icons fixes flakiness (elastic#62406) ...

kibanamachine · 2020-04-07T04:38:44Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

* [Reporting] Use a shim for server config (#62086) * config shim * simplify route register calls * switch to in-sync worker functions * fix tests * comment * fix set up config with defaults * reduce loc change * remove test for removed file * reportingconfigtype * revert changing executeJobFactory to synchronous * imports cleanup * Clean up some awaits * undo comment * clean up async * clean up imports * add warning logs for config defaults * Move around some config shim code * Register routes params take ReportingCore * usageCollection is an optional dependency * get config from ReportingCore for legacy (7.x) routes * fix parse errors * fix 7.x deltas Co-authored-by: Elastic Machine <[email protected]>

tsullivan changed the title ~~[Reporting] Add a shim for server config~~ [Reporting] Use a shim for server config Mar 31, 2020

tsullivan force-pushed the reporting/server-config-shim branch 2 times, most recently from 46b12fd to 58c12cb Compare April 1, 2020 21:45

tsullivan commented Apr 1, 2020

View reviewed changes

x-pack/legacy/plugins/reporting/server/legacy.ts Outdated Show resolved Hide resolved

tsullivan force-pushed the reporting/server-config-shim branch from 832cb9b to 79b1481 Compare April 2, 2020 04:54

tsullivan added 10 commits April 2, 2020 10:58

config shim

d3ebf22

simplify route register calls

b00aa71

switch to in-sync worker functions

a1e98d1

fix tests

61d0cf5

comment

b550692

fix set up config with defaults

b40b6a0

reduce loc change

4969682

remove test for removed file

7ee31fa

reportingconfigtype

9ccee60

revert changing executeJobFactory to synchronous

1705882

tsullivan force-pushed the reporting/server-config-shim branch from 79b1481 to 1705882 Compare April 2, 2020 18:38

imports cleanup

165ea32

tsullivan commented Apr 2, 2020

View reviewed changes

tsullivan added 4 commits April 2, 2020 12:34

Clean up some awaits

4098b82

undo comment

4dbb30e

clean up async

5bf7d13

clean up imports

1561725

tsullivan marked this pull request as ready for review April 2, 2020 22:08

tsullivan requested review from joelgriffith and a team April 2, 2020 22:08

tsullivan added (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:skip Skip the PR/issue when compiling release notes v7.8.0 labels Apr 2, 2020

tsullivan added the v8.0.0 label Apr 2, 2020

tsullivan added 4 commits April 2, 2020 15:22

add warning logs for config defaults

a9f7f60

Merge branch 'master' into reporting/server-config-shim

9792321

Move around some config shim code

c240335

Register routes params take ReportingCore

8d95558

tsullivan mentioned this pull request Apr 3, 2020

[Reporting/New Platform Migration] Use a new config service on server-side (re-do) #61696

Closed

usageCollection is an optional dependency

67acc88

tsullivan commented Apr 3, 2020

View reviewed changes

tsullivan mentioned this pull request Apr 3, 2020

[Reporting] Switch Serverside Config Wrapper to NP #62500

Merged

3 tasks

tsullivan requested review from afharo, pgayvallet and poffdeluxe April 3, 2020 19:38

tsullivan mentioned this pull request Apr 3, 2020

[Reporting] New Platform Migration #53898

Closed

19 tasks

joelgriffith approved these changes Apr 3, 2020

View reviewed changes

tsullivan merged commit f1f93d3 into elastic:master Apr 4, 2020

tsullivan mentioned this pull request Apr 4, 2020

[7.x] [Reporting] Use a shim for server config (#62086) #62540

Merged

tsullivan deleted the reporting/server-config-shim branch April 4, 2020 04:54

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Apr 7, 2020

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Apr 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Reporting] Use a shim for server config #62086

[Reporting] Use a shim for server config #62086

tsullivan commented Mar 31, 2020 •

edited

Loading

tsullivan Apr 2, 2020

tsullivan Apr 2, 2020

tsullivan Apr 2, 2020

tsullivan Apr 2, 2020

tsullivan Apr 3, 2020

kibanamachine commented Apr 3, 2020

joelgriffith left a comment

kibanamachine commented Apr 7, 2020

[Reporting] Use a shim for server config #62086

[Reporting] Use a shim for server config #62086

Conversation

tsullivan commented Mar 31, 2020 • edited Loading

Summary

Checklist

For maintainers

tsullivan Apr 2, 2020

Choose a reason for hiding this comment

tsullivan Apr 2, 2020

Choose a reason for hiding this comment

tsullivan Apr 2, 2020

Choose a reason for hiding this comment

tsullivan Apr 2, 2020

Choose a reason for hiding this comment

tsullivan Apr 3, 2020

Choose a reason for hiding this comment

kibanamachine commented Apr 3, 2020

💚 Build Succeeded

History

joelgriffith left a comment

Choose a reason for hiding this comment

kibanamachine commented Apr 7, 2020

tsullivan commented Mar 31, 2020 •

edited

Loading