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

fixing issues with maximum documents and maximum size being set #31998

Merged

Conversation

bmcconaghy
Copy link
Contributor

Closes #31997

This PR fixes a few issues with ILM UI and max_size/max_docs including:

  • defaults were being used for existing policies for max size/max age when the user had not defined those
  • only max size or max docs could be defined, now the UI supports both
  • only gb was supported as a size unit, now mb is as well

@bmcconaghy bmcconaghy added v7.0.0 Feature:ILM v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v6.7.0 v7.2.0 labels Feb 26, 2019
@bmcconaghy bmcconaghy requested a review from sebelga February 26, 2019 08:04
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@sebelga sebelga left a comment

Choose a reason for hiding this comment

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

LGTM, tested locally and works as expected!

id={`${PHASE_HOT}-${PHASE_ROLLOVER_MAX_DOCUMENTS}`}
value={phaseData[PHASE_ROLLOVER_MAX_DOCUMENTS]}
onChange={async e => {
await setPhaseData(
Copy link
Contributor

@sebelga sebelga Feb 26, 2019

Choose a reason for hiding this comment

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

The store/actions/policies.js that contains setPhaseData () seems to be synchronous. Do we need to await here and in other onChange handlers in this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, left over from a refactor, good catch.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@bmcconaghy
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@bmcconaghy bmcconaghy merged commit a382b49 into elastic:master Feb 26, 2019
@bmcconaghy bmcconaghy deleted the fix_issues_with_max_size_and_docs branch February 26, 2019 15:23
bmcconaghy added a commit to bmcconaghy/kibana that referenced this pull request Feb 26, 2019
…tic#31998)

* fixing issues with maximum documents and maximum size being set

* removing defaults from ES policy load and removing unneeded propTypes

* fixing react warnings about controlled -> uncontrolled

* fixing tests

* more fixes for react controlled -> uncontrolled errors

* better fix for react warnings through empty default phases

* removing unused translation

* removing unnecessary async/awaits in onChange handlers
bmcconaghy added a commit to bmcconaghy/kibana that referenced this pull request Feb 26, 2019
…tic#31998)

* fixing issues with maximum documents and maximum size being set

* removing defaults from ES policy load and removing unneeded propTypes

* fixing react warnings about controlled -> uncontrolled

* fixing tests

* more fixes for react controlled -> uncontrolled errors

* better fix for react warnings through empty default phases

* removing unused translation

* removing unnecessary async/awaits in onChange handlers
bmcconaghy added a commit that referenced this pull request Feb 26, 2019
…) (#32039)

* fixing issues with maximum documents and maximum size being set

* removing defaults from ES policy load and removing unneeded propTypes

* fixing react warnings about controlled -> uncontrolled

* fixing tests

* more fixes for react controlled -> uncontrolled errors

* better fix for react warnings through empty default phases

* removing unused translation

* removing unnecessary async/awaits in onChange handlers
bmcconaghy added a commit to bmcconaghy/kibana that referenced this pull request Feb 26, 2019
…tic#31998)

* fixing issues with maximum documents and maximum size being set

* removing defaults from ES policy load and removing unneeded propTypes

* fixing react warnings about controlled -> uncontrolled

* fixing tests

* more fixes for react controlled -> uncontrolled errors

* better fix for react warnings through empty default phases

* removing unused translation

* removing unnecessary async/awaits in onChange handlers
bmcconaghy added a commit that referenced this pull request Feb 26, 2019
…) (#32040)

* fixing issues with maximum documents and maximum size being set

* removing defaults from ES policy load and removing unneeded propTypes

* fixing react warnings about controlled -> uncontrolled

* fixing tests

* more fixes for react controlled -> uncontrolled errors

* better fix for react warnings through empty default phases

* removing unused translation

* removing unnecessary async/awaits in onChange handlers
bmcconaghy added a commit that referenced this pull request Feb 26, 2019
…) (#32041)

* fixing issues with maximum documents and maximum size being set

* removing defaults from ES policy load and removing unneeded propTypes

* fixing react warnings about controlled -> uncontrolled

* fixing tests

* more fixes for react controlled -> uncontrolled errors

* better fix for react warnings through empty default phases

* removing unused translation

* removing unnecessary async/awaits in onChange handlers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:ILM Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v6.7.0 v7.0.0 v7.2.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ILM UI does not handle max docs and max size properly
3 participants