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

Fix validation to parse partitions as ints first #2512

Merged
merged 4 commits into from
Jun 24, 2024

Conversation

aindriu-aiven
Copy link
Contributor

@aindriu-aiven aindriu-aiven commented Jun 21, 2024

Linked issue

Resolves: #2508

What kind of change does this PR introduce?

  • Bug fix
  • New feature
  • Refactor
  • Docs update
  • CI update

What is the current behavior?

Strings are compared to each other for default and max paritions and replication

What is the new behavior?

Strings are parsed to ints then compared to each other for default and max paritions and replication

Other information

Additional changes, explanations of the approach taken, unresolved issues, necessary follow ups, etc.

Requirements (all must be checked before review)

  • The pull request title follows our guidelines
  • Tests for the changes have been added (if relevant)
  • The latest changes from the main branch have been pulled
  • pnpm lint has been run successfully

@@ -705,6 +705,11 @@ app.controller("envsCtrl", function($scope, $http, $location, $window) {

$scope.addNewEnv = function() {

var defRepFactor = parseInt($scope.addNewEnv.defrepfctr,10)
var maxRepFactor = parseInt($scope.addNewEnv.maxrepfctr,10)
var defPartitionFactor = parseInt($scope.addNewEnv.defparts,10)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
var defPartitionFactor = parseInt($scope.addNewEnv.defparts,10)
var defPartitions = parseInt($scope.addNewEnv.defparts,10)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well that was a silly mistake to make gimme two minutes and I'll fix it

var defRepFactor = parseInt($scope.addNewEnv.defrepfctr,10)
var maxRepFactor = parseInt($scope.addNewEnv.maxrepfctr,10)
var defPartitionFactor = parseInt($scope.addNewEnv.defparts,10)
var maxPartitionFactor = parseInt($scope.addNewEnv.defmaxparts,10)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
var maxPartitionFactor = parseInt($scope.addNewEnv.defmaxparts,10)
var maxPartitions = parseInt($scope.addNewEnv.defmaxparts,10)

Signed-off-by: Aindriu Lavelle <[email protected]>
Copy link
Contributor

@muralibasani muralibasani left a comment

Choose a reason for hiding this comment

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

LGTM, thx

@aindriu-aiven aindriu-aiven merged commit 841c134 into main Jun 24, 2024
16 checks passed
@aindriu-aiven aindriu-aiven deleted the fix-validation-on-env-create-modify branch June 24, 2024 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validation of default and maximum partitions for environment
2 participants