Skip to content
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

Prevent creating a job in the reporting index when the type is immediate #35011

Conversation

tsullivan
Copy link
Member

Summary

In the implementation of "immediate" generation of CSV from a saved object, work is done in the "immediate" route handler to craft a job document for the .reporting index, and make sure the generated CSV contents are stored in the index for later re-download.

This has a lot of downsides:

  • A wrapper for generateCsv would be called in 2 places: createJob (immediate only) and executeJob (non-immediate only)
  • There was a duplicate of the generated content in the reporting job document, a problem which was going to need to be solved

This PR has the "immediate" route handler calls createJob and executeJob directly, not through the helper function that uses ESQueue to create the document in ES. Those functions analyze the request and look up the saved object details for metadata used for generating the content immediately - which now will only happen from executeJob

@tsullivan tsullivan added review (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead labels Apr 12, 2019
@tsullivan tsullivan requested a review from joelgriffith April 12, 2019 17:48
@@ -6,8 +6,6 @@

import { badRequest } from 'boom';
import { Request } from 'hapi';
// @ts-ignore
import { createTaggedLogger } from '../../../../server/lib';
Copy link
Member Author

Choose a reason for hiding this comment

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

Cleanup, this was invalid but it had ts-ignore

@@ -20,7 +20,7 @@ export interface SavedSearchGeneratorResult {

export interface CsvResultFromSearch {
type: string;
result: SavedSearchGeneratorResult | null;
result: SavedSearchGeneratorResult;
Copy link
Member Author

Choose a reason for hiding this comment

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

It made things complicated to try to support this being null, since the value gets passed in h.response in the route handler, and h.response does not accept null per TS types.

That is why, further down, the code will check if this is null and convert to undefined

@tsullivan tsullivan force-pushed the feature/reporting/csv-export-panel-action-no-job-create-for-immediate branch 2 times, most recently from f9d25de to eff1dbb Compare April 12, 2019 18:03
@tsullivan tsullivan force-pushed the feature/reporting/csv-export-panel-action-no-job-create-for-immediate branch from eff1dbb to 0bdbf93 Compare April 12, 2019 18:09
@@ -0,0 +1,51 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
Copy link
Member Author

Choose a reason for hiding this comment

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

Added this change for CI to pass. These changes come from #34972

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

…/reporting/csv-export-panel-action-no-job-create-for-immediate
@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@joelgriffith joelgriffith force-pushed the feature/reporting/csv-export-panel-action branch from 3334140 to 4446428 Compare April 15, 2019 20:16
@joelgriffith joelgriffith merged commit 61c1c1b into elastic:feature/reporting/csv-export-panel-action May 3, 2019
@tsullivan tsullivan deleted the feature/reporting/csv-export-panel-action-no-job-create-for-immediate branch August 15, 2019 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants