Skip to content

[Reporting] remove legacy pdf shim#121369

Merged
tsullivan merged 6 commits intoelastic:mainfrom
tsullivan:reporting/pdf-shim-removal
Dec 21, 2021
Merged

[Reporting] remove legacy pdf shim#121369
tsullivan merged 6 commits intoelastic:mainfrom
tsullivan:reporting/pdf-shim-removal

Conversation

@tsullivan
Copy link
Copy Markdown
Member

@tsullivan tsullivan commented Dec 16, 2021

Summary

Closes #120192

Checklist

Delete any items that are not applicable to this PR.

For maintainers

Release note:

Breaking change: POST URLs generated for PDF reports in Kibana 6.2 will no longer work. Please regenerate the POST URLs that you need for automating reports.

@tsullivan tsullivan requested review from a team as code owners December 16, 2021 00:42
@tsullivan tsullivan added Breaking Change release_note:breaking zDeprecated Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead Team:AppServicesUx technical debt Improvement of the software architecture and operational architecture labels Dec 16, 2021
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-app-services (Team:AppServicesUx)

@elasticmachine
Copy link
Copy Markdown
Contributor

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

objects: relativeUrls.map((u) => ({ relativeUrl: u })),
// return the payload
return {
...jobParams,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I noticed this should have isDeprecated: true since we have a V2 type for this. I'm leaving that as a separate issue: #121365

"max_attempts": undefined,
"meta": Object {
"isDeprecated": false,
"isDeprecated": undefined,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is not exactly correct (should be true since there is a V2 type), but is related to #121365

import * as GenerationUrls from '../services/generation_urls';
import { ReportingUsageStats } from '../services/usage';

// These all have the domain name portion stripped out. The api infrastructure assumes it when we post to it anyhow.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Review note: Now that the shim is removed, this URLs are only used in this file, so I removed the entire GenerationUrls service to inline the URLs in this file.

Copy link
Copy Markdown
Contributor

@vadimkibana vadimkibana left a comment

Choose a reason for hiding this comment

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

Core review only, LGTM.

@tsullivan
Copy link
Copy Markdown
Member Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@tsullivan tsullivan merged commit d6fe658 into elastic:main Dec 21, 2021
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Dec 21, 2021
@tsullivan tsullivan deleted the reporting/pdf-shim-removal branch December 21, 2021 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Breaking Change release_note:breaking technical debt Improvement of the software architecture and operational architecture v8.1.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] Compatibility shim for generating PDF reports using 6.2 URLs can be removed.

5 participants