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

[Reporting] rename interfaces to align with task manager integration #76716

Merged

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Sep 3, 2020

Summary

This PR sweeps through the Reporting server code and renames interfaces to align with the architectural design of #64853

Reasoning behind the changes: we should get away from the "scheduling" terminology as the name of the process where user input data gets converted into a report document.

When a report executes, it relies on a "job" that been "created" to contain a "payload." The payload is the user input + a string of encrypted headers. Scheduled reports will rely on the created job as well: it will be the task instance parameters.

Therefore, "creating a job" is different from "scheduling a task."

This PR goes back to naming the interfaces of this process to "create job" terminology.

  • Moves screenshot interfaces to screenshot folder
  • Organize imports

@tsullivan tsullivan marked this pull request as ready for review September 3, 2020 22:01
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-reporting-services (Team:Reporting Services)

@@ -5,16 +5,16 @@
*/

import { cryptoFactory } from '../../lib';
import { CreateJobFn, ScheduleTaskFnFactory } from '../../types';
import { CreateJobFn, CreateJobFnFactory } from '../../types';
Copy link
Contributor

Choose a reason for hiding this comment

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

WDYT about keeping the task nomenclature here? CreateTaskFnFactory?

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.

Not sure if we need to move job-based names to be task-based, or if there's something about the taxonomy of our objects that requires tasks vs jobs?

NM: read your description :)

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, seems to align well with the diagram you shared

@tsullivan tsullivan mentioned this pull request Sep 3, 2020
7 tasks
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

@tsullivan tsullivan merged commit e4464a1 into elastic:master Sep 4, 2020
@tsullivan tsullivan deleted the reporting/rename-server-types-createjob branch September 4, 2020 04:20
tsullivan added a commit to tsullivan/kibana that referenced this pull request Sep 4, 2020
…elastic#76716)

* [Reporting] rename interfaces for schedule task

* move screenshot types to their local

* organize imports

* remove schedule terminology

* revert comment

* fix ts

* add newline
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 4, 2020
…rok/new-patterns-component-use-array

* 'master' of github.com:elastic/kibana: (75 commits)
  Remove legacy ui-apps-mixin (elastic#76604)
  remove unused test_utils (elastic#76528)
  [ML] Functional tests - add UI permission tests (elastic#76368)
  [APM] @ts-error -> @ts-expect-error (elastic#76492)
  [APM] Avoid negative offset for error marker on timeline (elastic#76638)
  [Reporting] rename interfaces to align with task manager integration (elastic#76716)
  Revert back ESO migration for alerting, added try/catch logic to avoid failing Kibana on start (elastic#76220)
  Test reverting "Add plugin status API (elastic#75819)" (elastic#76707)
  [Security Solution][Detections] Removes ML Job Settings SIEM copy and fixes link to ML app for creating custom jobs (elastic#76595)
  [Maps] remove region/coordinate-maps visualizations from sample data (elastic#76399)
  [DOCS] Dashboard-first docs refresh (elastic#76194)
  Updated ServiceNow description in docs and actions management UI to contains correct info (elastic#76344)
  [DOCS] Identifies cloud settings in reporting (elastic#76691)
  [Security Solution] Refactor timeline details to use search strategy (elastic#75591)
  es-archiver: Drop invalid index settings, support --query flag  (elastic#76522)
  [DOCS] Identifies graph settings available on cloud (elastic#76661)
  Add more info about a11y tests (elastic#76045)
  [data.search.SearchSource] Remove legacy ES client APIs. (elastic#75943)
  [release notes] automatically retry on Github API 5xx errors (elastic#76447)
  [es_ui_shared] Fix eslint exhaustive deps rule (elastic#76392)
  ...
tsullivan added a commit that referenced this pull request Sep 5, 2020
…ation (#76716) (#76737)

* [Reporting] rename interfaces to align with task manager integration (#76716)

* [Reporting] rename interfaces for schedule task

* move screenshot types to their local

* organize imports

* remove schedule terminology

* revert comment

* fix ts

* add newline

* coment

* fix tests
@sophiec20 sophiec20 added the (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants