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

[licensing] Remove unnecessary refresh calls #194499

Merged

Conversation

afharo
Copy link
Member

@afharo afharo commented Sep 30, 2024

Summary

Removing the usage of licensing.refresh() as it forces a new retrieval of the license when we only need the current one.

For maintainers

@afharo afharo force-pushed the licensing/remove-unnecessary-force-refreshes branch from b61721e to efa4864 Compare October 1, 2024 07:55
@afharo afharo self-assigned this Oct 1, 2024
@afharo afharo added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Cloud Security Cloud Security team related labels Oct 1, 2024
@afharo afharo requested a review from a team October 1, 2024 11:42
@afharo afharo marked this pull request as ready for review October 1, 2024 11:43
@afharo afharo requested review from a team as code owners October 1, 2024 11:43
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

@afharo afharo linked an issue Oct 1, 2024 that may be closed by this pull request
Copy link
Contributor

@gsoldevila gsoldevila left a comment

Choose a reason for hiding this comment

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

Core changes LGTM!

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

kibana-presentation changes LGTM
code review only

@@ -134,6 +134,7 @@ export class LicensingPlugin implements Plugin<LicensingPluginSetup, LicensingPl
}
return {
refresh: this.refresh,
getLicense: async () => await firstValueFrom(this.license$!),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is worth updating docs in the Readme, right now docs say refresh: () => Promise<ILicense> allows a plugin to enforce license retrieval. which reads misleading

Copy link
Member Author

Choose a reason for hiding this comment

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

great point! I'll update the readme

Copy link
Member Author

Choose a reason for hiding this comment

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

Adressed in dab01c7 (#194499)

Copy link
Contributor

@maxcold maxcold left a comment

Choose a reason for hiding this comment

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

cloud_security_posture and cloud_defend changes lgtm, test cloud_security_posture a bit manually.

@afharo afharo enabled auto-merge (squash) October 3, 2024 13:49
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
cloudDefend 101.1KB 101.1KB +3.0B
cloudSecurityPosture 506.9KB 506.9KB +3.0B
total +6.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
licensing 11.6KB 11.7KB +66.0B
maps 54.4KB 54.4KB +3.0B
mapsEms 5.8KB 5.8KB +3.0B
total +72.0B
Unknown metric groups

API count

id before after diff
licensing 117 119 +2

References to deprecated APIs

id before after diff
mapsEms 2 0 -2

Unreferenced deprecated APIs

id before after diff
licensing 3 4 +1

History

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

cc @afharo

@afharo afharo merged commit f3f53e0 into elastic:main Oct 3, 2024
22 checks passed
@afharo afharo deleted the licensing/remove-unnecessary-force-refreshes branch October 3, 2024 15:42
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11165182532

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 3, 2024
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 3, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [[licensing] Remove unnecessary refresh calls
(#194499)](#194499)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Alejandro Fernández
Haro","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-03T15:42:39Z","message":"[licensing]
Remove unnecessary refresh calls
(#194499)","sha":"f3f53e054237087aab8590084cb7c8c10972427c","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Core","Team:Presentation","release_note:skip","v9.0.0","Team:Cloud
Security","backport:prev-minor"],"title":"[licensing] Remove unnecessary
refresh
calls","number":194499,"url":"https://github.com/elastic/kibana/pull/194499","mergeCommit":{"message":"[licensing]
Remove unnecessary refresh calls
(#194499)","sha":"f3f53e054237087aab8590084cb7c8c10972427c"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194499","number":194499,"mergeCommit":{"message":"[licensing]
Remove unnecessary refresh calls
(#194499)","sha":"f3f53e054237087aab8590084cb7c8c10972427c"}}]}]
BACKPORT-->

Co-authored-by: Alejandro Fernández Haro <[email protected]>
tiansivive pushed a commit to tiansivive/kibana that referenced this pull request Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Licensing] Expose getter in the plugin's start contract
7 participants