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

Make migration thread count counfigurable #19794

Merged
merged 7 commits into from
Jul 9, 2024
Merged

Make migration thread count counfigurable #19794

merged 7 commits into from
Jul 9, 2024

Conversation

gally47
Copy link
Contributor

@gally47 gally47 commented Jul 1, 2024

Motivation and Context

fixes #19792

Screenshots (if appropriate):

Screenshot 2024-07-01 at 19 04 09

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

@gally47 gally47 self-assigned this Jul 1, 2024
Copy link
Contributor

@moesterheld moesterheld left a comment

Choose a reason for hiding this comment

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

Works in general. Thank you for adding.
However, I cannot remove the value in the input field using the keyboard/mouse, I can only add to it.
Also, one small textual change.

@gally47
Copy link
Contributor Author

gally47 commented Jul 8, 2024

Works in general. Thank you for adding. However, I cannot remove the value in the input field using the keyboard/mouse, I can only add to it. Also, one small textual change.

@moesterheld the value is not supposed to be removed you can only replace the default value (4) with any positive integer.

@moesterheld
Copy link
Contributor

Works in general. Thank you for adding. However, I cannot remove the value in the input field using the keyboard/mouse, I can only add to it. Also, one small textual change.

@moesterheld the value is not supposed to be removed you can only replace the default value (4) with any positive integer.

yep. never mind, I retried it and it behaves consistently with all other numeric input fields in Graylog.
I wanted to double click the input and write another value into it, however it didn't select the value so I could directly replace it. Also, backspace won't work, which feels a little weird. I think the behavior of the numeric input could be improved in general, but it's ok for now.

Copy link
Contributor

@ousmaneo ousmaneo left a comment

Choose a reason for hiding this comment

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

It works as expected. I have a small comment.

value = e.target.value;

if (e.target.name === 'threads') {
value = Number(e.target.value) < 1 ? DEFAULT_THREADS_COUNT : Number(e.target.value);
Copy link
Contributor

Choose a reason for hiding this comment

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

We could use the getValueFromInput from the FormUtils.

@gally47 gally47 requested a review from ousmaneo July 8, 2024 12:45
Copy link
Contributor

@todvora todvora left a comment

Choose a reason for hiding this comment

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

LGTM!

@todvora todvora merged commit a8b1ea8 into master Jul 9, 2024
6 checks passed
@todvora todvora deleted the threads-count branch July 9, 2024 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make rem.migration thread count counfigurable
4 participants