Skip to content

[Reporting/Typescript] Fix types for JobParamsPDF, BrowserConfig, ChromiumDriverFactory#45209

Merged
tsullivan merged 8 commits intoelastic:masterfrom
tsullivan:reporting/pdf-params-type-fix
Sep 13, 2019
Merged

[Reporting/Typescript] Fix types for JobParamsPDF, BrowserConfig, ChromiumDriverFactory#45209
tsullivan merged 8 commits intoelastic:masterfrom
tsullivan:reporting/pdf-params-type-fix

Conversation

@tsullivan
Copy link
Member

@tsullivan tsullivan commented Sep 9, 2019

Summary

Closes #42421

This change provides type definition fixes for a few types that have been brought in for Typescript conversion of Reporting. The changes here are related to Typescriptification happening since 7.4.0 development.

Addresses:

@tsullivan tsullivan added chore zDeprecated Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead v8.0.0 v7.5.0 release_note:skip Skip the PR/issue when compiling release notes labels Sep 9, 2019
@tsullivan tsullivan force-pushed the reporting/pdf-params-type-fix branch from fc05794 to 961ce14 Compare September 9, 2019 21:49
flags.push('--no-sandbox');
}

// TODO remove conditional
Copy link
Member Author

Choose a reason for hiding this comment

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

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

server: string;
bypass?: string[];
};
verboseLogging?: boolean;
Copy link
Member Author

Choose a reason for hiding this comment

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

This would be removed in #45479

import { LevelLogger } from '../lib/level_logger';
import { KbnServer } from '../../types';
import { PLUGIN_ID } from '../../common/constants';
import { HeadlessChromiumDriverFactory } from './chromium/driver_factory';
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated: I've been thinking we might want to reuse this factory, potentially, for scheduled reports. Not sure about your thoughts on that.

Copy link
Member Author

Choose a reason for hiding this comment

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

++ this is one of the guts that we can reuse, as it's not specific to scheduling

Copy link
Contributor

@joelgriffith joelgriffith left a comment

Choose a reason for hiding this comment

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

LGTM, code checked

@tsullivan tsullivan merged commit 4a9dc66 into elastic:master Sep 13, 2019
@tsullivan tsullivan deleted the reporting/pdf-params-type-fix branch September 13, 2019 17:54
tsullivan added a commit to tsullivan/kibana that referenced this pull request Sep 13, 2019
…omiumDriverFactory (elastic#45209)

* [Reporting] Correct PDF Params type definition

* Fix types for BrowserConfig, ChromiumDriverFactory

* remove jobparams type

* spot fixes

* fix guard in compat shim

* fix legacy functional test

* fix jest test
tsullivan added a commit that referenced this pull request Sep 13, 2019
…omiumDriverFactory (#45209) (#45661)

* [Reporting] Correct PDF Params type definition

* Fix types for BrowserConfig, ChromiumDriverFactory

* remove jobparams type

* spot fixes

* fix guard in compat shim

* fix legacy functional test

* fix jest test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore release_note:skip Skip the PR/issue when compiling release notes v7.5.0 v8.0.0 zDeprecated Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Reporting] Generate PNG API should not allow multi-page request

3 participants