cloud: add isInTrial helper function#239210
Conversation
|
cc: @afharo & @jloleysens |
a5bfe92 to
dc06931
Compare
|
Pinging @elastic/fleet (Team:Fleet) |
jbudz
left a comment
There was a problem hiding this comment.
packages/kbn-optimizer/limits.yml LGTM
1189647 to
7eabc4c
Compare
|
@elasticmachine merge upstream |
jloleysens
left a comment
There was a problem hiding this comment.
Approving to unblock progress. Thanks for cleaning this up @TattdCodeMonkey.
I'd like to get your thoughts about removing trialEndDate from the setup contract before merging!
| }); | ||
|
|
||
| it('should set the feedback button visibility to "false" for deployment in trial', async () => { | ||
| it('should set the feedback button visibility to "false" for deployment in trial via endDate', async () => { |
There was a problem hiding this comment.
| it('should set the feedback button visibility to "false" for deployment in trial via endDate', async () => { | |
| it('should set the feedback button visibility to "false" for deployment in trial', async () => { |
Seems like we're removing knowledge of "endDate"?
There was a problem hiding this comment.
yes, other plugins should just use the function and let the cloud plugin handle all the logic around endDate.
|
|
||
| expect(setup.getInTrial()).toBe(false); | ||
| }); | ||
| it('is `false` when `serverless.in_trial` & `trial_end_date` are not set', () => { |
There was a problem hiding this comment.
What happens when both serverless.in_trial and trial_end_date are set? IMO we should throw and remove all serverless.in_trial test cases from this set of tests and scope those to the serverless:true config tests below.
There was a problem hiding this comment.
@jloleysens serverless.in_trial will override any trial_end_date for the purpose of getInTrial.
I will remove all serverless.in_trial case from here.
I can add code to throw if both are set, but that makes me a little uneasy since both of these config values are set from other systems. I feel like it's more likely someone will break Kibana unwittingly.
There was a problem hiding this comment.
remove all serverless.in_trial test cases from this set of tests and scope those to the serverless:true config tests below.
The tests below are testing the start interface. These tests are testing setup. Technically we're exposing the same function, but I'd prefer to have the function tests for both contracts.
I will move the serverless.in_trial test cases into the is serverless enabled section.
| cloudDefaultPort: decodedId?.defaultPort, | ||
| isCloudEnabled, | ||
| trialEndDate: this.config.trial_end_date ? new Date(this.config.trial_end_date) : undefined, | ||
| trialEndDate: this.trialEndDate, |
There was a problem hiding this comment.
Can we remove this from the setup contract for now? Or does someone need the actual date?
There was a problem hiding this comment.
We could, the actual date could be useful in the frontend if we wanted to show something like "5 days left in trial" which I've seen product interested in before.
But currently it is not used like that. So I'm fine removing it if we things thats the best option.
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
|
Oh, and can we test this on serverless before merging, I added the project deploy labels! |
Added the getInTrial helper function to the cloud plugin setup & start interfaces that allow checking if the deployment or serverless project are in trial based on either the trial_end_date or serverless.in_trial config values being set. Adding a function allows the date comparison to be handled by the cloud plugin and possibly updated in the future without consumers needing to know the details of how we verify if the cluster is within a trial cloud organization.
54419eb to
14b5783
Compare
Worked on reducing page load bundle size for cloud, but still needed to increase the limit slightly. While working on that change I also added some extra code to handle if the configured `trial_end_date` does not parse, which should never happen but is possible so I ultimately decided to make sure it was handled and logged.
14b5783 to
122d8af
Compare
⏳ Build in-progress
Failed CI StepsTest Failures
History
|
## Summary Added the isInTrial helper function to the cloud plugin setup & start interfaces that allow checking if the deployment or serverless project are in trial based on either the trial_end_date or serverless.in_trial config values being set. Adding a function allows the date comparison to be handled by the cloud plugin and possibly updated in the future without consumers needing to know the details of how we verify if the cluster is within a trial cloud organization. This is a follow-up to [elastic#232703](elastic#232703) where I added the `serverless.in_trial` config value. In subsequent PRs it was discussed we should have a helper on the cloud function to give uniform way to access "in trial". ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] ~Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)~ - [ ] ~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials~ - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] ~If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~ - [ ] ~This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations.~ - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] ~The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)~ - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels.
## Summary Added the isInTrial helper function to the cloud plugin setup & start interfaces that allow checking if the deployment or serverless project are in trial based on either the trial_end_date or serverless.in_trial config values being set. Adding a function allows the date comparison to be handled by the cloud plugin and possibly updated in the future without consumers needing to know the details of how we verify if the cluster is within a trial cloud organization. This is a follow-up to [elastic#232703](elastic#232703) where I added the `serverless.in_trial` config value. In subsequent PRs it was discussed we should have a helper on the cloud function to give uniform way to access "in trial". ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] ~Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)~ - [ ] ~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials~ - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] ~If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~ - [ ] ~This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations.~ - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] ~The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)~ - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels.
## Summary Added the isInTrial helper function to the cloud plugin setup & start interfaces that allow checking if the deployment or serverless project are in trial based on either the trial_end_date or serverless.in_trial config values being set. Adding a function allows the date comparison to be handled by the cloud plugin and possibly updated in the future without consumers needing to know the details of how we verify if the cluster is within a trial cloud organization. This is a follow-up to [elastic#232703](elastic#232703) where I added the `serverless.in_trial` config value. In subsequent PRs it was discussed we should have a helper on the cloud function to give uniform way to access "in trial". ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] ~Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)~ - [ ] ~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials~ - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] ~If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~ - [ ] ~This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations.~ - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] ~The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)~ - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels.
Summary
Added the isInTrial helper function to the cloud plugin setup & start interfaces that allow checking if the deployment or serverless project are in trial based on either the trial_end_date or serverless.in_trial config values being set. Adding a function allows the date comparison to be handled by the cloud plugin and possibly updated in the future without consumers needing to know the details of how we verify if the cluster is within a trial cloud organization.
This is a follow-up to #232703 where I added the
serverless.in_trialconfig value. In subsequent PRs it was discussed we should have a helper on the cloud function to give uniform way to access "in trial".Checklist
Check the PR satisfies following conditions.
Reviewers should verify this PR satisfies this list as well.
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportDocumentation was added for features that require explanation or tutorialsIf a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker listThis was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. Therelease_note:breakinglabel should be applied in these situations.The PR description includes the appropriate Release Notes section, and the correctrelease_note:*label is applied per the guidelinesbackport:*labels.