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

[Rollups] Update rollup badges in Data view form #187235

Merged
merged 5 commits into from
Jul 2, 2024

Conversation

ElenaStoeva
Copy link
Contributor

@ElenaStoeva ElenaStoeva commented Jul 1, 2024

Closes #186663

Summary

Follow-up to #186321. This PR updates the Rollup badges in the Data view create form so that they show that Rollup is deprecated.

Before: Screenshot 2024-07-01 at 11 48 21
Now: Screenshot 2024-07-01 at 17 50 48 Screenshot 2024-07-01 at 17 50 54 Screenshot 2024-07-01 at 17 50 59

How to test:

  1. Start Es and Kibana
  2. Add the sample data "Sample web logs"
  3. Elasticsearch only allows creating a rollup job if there is an existing rollup usage in the cluster. To simulate rollup usage, create a mock rollup index through Console:
PUT /mock_rollup_index
{
  "mappings": {
    "_meta": {
      "_rollup": {
        "id": "logs_job"
      }
    }
  }
}
  1. Create a sample rollup job through Console:
PUT _rollup/job/logs_job
{
  "id": "logs_job",
  "index_pattern": "kibana_sample_data_logs",
  "rollup_index": "rollup_logstash",
  "cron": "* * * * * ?",
  "page_size": 1000,
  "groups": {
    "date_histogram": {
      "interval": "60m",
      "delay": "7d",
      "time_zone": "UTC",
      "field": "@timestamp"
    },
    "terms": {
      "fields": [
        "geo.src",
        "machine.os.keyword"
      ]
    },
    "histogram": {
      "interval": "1003",
      "fields": [
        "bytes",
        "memory"
      ]
    }
  }
}
  1. Delete the mock rollup index since it causes issues for the rollup API that we use to fetch rollup indices:
    DELETE /mock_rollup_index
  2. Navigate to Stack Management -> Data Views and start creating a new data view.
  3. Verify that the rollup badges are updated.

@ElenaStoeva ElenaStoeva added Feature:Data Views Data Views code and UI - index patterns before 8.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes Feature:Rollups labels Jul 1, 2024
@ElenaStoeva ElenaStoeva self-assigned this Jul 1, 2024
@ElenaStoeva
Copy link
Contributor Author

/ci

@ElenaStoeva ElenaStoeva marked this pull request as ready for review July 1, 2024 16:52
@ElenaStoeva ElenaStoeva requested a review from a team as a code owner July 1, 2024 16:52
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-management (Team:Kibana Management)

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
dataViewEditor 81 86 +5

Async chunks

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

id before after diff
dataViewEditor 48.8KB 49.5KB +774.0B
dataViews 5.0KB 5.0KB +13.0B
total +787.0B

History

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

cc @ElenaStoeva

Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

Changes LGTM, thanks!

It might be better to change Beta badge color from the bright pink to a gray color.

defaultMessage="Deprecated"
/>
</EuiBadge>
</RollupDeprecationTooltip>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we also update i18n indexPatternEditor.typeSelect.rollup string on Line 103 with Rollup (deprecated)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review @jughosta! Yes, it makes sense to me to rename the rollup option to Rollup (deprecated). Regarding changing the Beta badge color to gray, @jovana-andjelkovic do you have any opinion on this?

Screenshot 2024-07-02 at 16 42 02

Choose a reason for hiding this comment

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

I agree with renaming the rollup option as well 👍

@ElenaStoeva Given that we're using gray color for Index in this context, I would avoid using the same for Beta.

I took a look a look at EUI specs and we don't have guidelines on when which version of the Beta badge is used, and it looks like we're not using the designated beta badge here anyways 🤔

Overall, I do see the point in making Beta badge stand out less, but given the usage of Rollups, I would say let's keep the pink color as is.

cc @alisonelizabeth @formgeist @tylerperk - no action needed, just to be aware of changes.

@ElenaStoeva ElenaStoeva enabled auto-merge (squash) July 2, 2024 16:10
@ElenaStoeva ElenaStoeva merged commit 628994e into elastic:main Jul 2, 2024
19 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jul 2, 2024
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
dataViewEditor 81 86 +5

Async chunks

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

id before after diff
dataViewEditor 48.8KB 49.5KB +787.0B
dataViews 5.0KB 5.0KB +13.0B
total +800.0B

cc @ElenaStoeva

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Data Views Data Views code and UI - index patterns before 8.0 Feature:Rollups release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Rollup] Update rollup badges in Data view create form
6 participants