Feature: Remove arbitrary maximum integer values. Implements #101 #102
Conversation
| @@ -182,7 +182,7 @@ const ConfigurationForm = () => { | |||
| autoCorrect="off" | |||
| autoCapitalize="none" | |||
| min={1} | |||
| max={9999} | |||
| max={Number.MAX_SAFE_INTEGER} | |||
There was a problem hiding this comment.
I am not sure, postgresql exists server, which have more, than 9999 CPUs.
There was a problem hiding this comment.
An arbitrary limit on memory forced me to fork in order to use the tool. The UI code didn't handle not having a max value, so I set everything to the maximum allowable value. I don't necessarily think it makes sense to add arbitrary limits, as they cause issues for users, and aren't likely to cause typos/errors as they're implemented (since 9999 is "too high" anyway).
| @@ -203,7 +203,7 @@ const ConfigurationForm = () => { | |||
| autoCorrect="off" | |||
| autoCapitalize="none" | |||
| min={20} | |||
| max={9999} | |||
| max={Number.MAX_SAFE_INTEGER} | |||
There was a problem hiding this comment.
1000 connections even dont make sense, so I dont see reason for this
There was a problem hiding this comment.
Suppose I have a 4 socket EPYC server, with 768 cores and 2TB of ram.... At 15MB per connection = ~83,200 theoretical max connections.
I'm mostly concerned with memory, however, I don't see a logical reason to enforce a maximum on any of these values. (How did we land on 9999?)
|
Let put just |
jmealo
force-pushed
the
feat/issue-101
branch
from
d3d4215 to
b9e7745
Compare
jmealo
force-pushed
the
feat/issue-101
branch
from
b9e7745 to
fb96bc8
Compare
|
@le0pard: Thank you. That sounds reasonable to me. I added a constant in the config for It occured to me that allowing decimal inputs when memory units are GB would also make a ton of sense. I still require MB to be an integer, but allow decimal for GB. What do you think? |
|
Problem with decimal, that different locales using different separator, so I better skip this for now. Just don't forget fix |
|
@le0pard: I just noticed that! I tried to do it in a locale-aware way and it was giving me some trouble. I'll back that out. |
|
@le0pard: Ok, I removed decimal support, everything looks good/consistent here with the regex/constant-usage to me. |
|
thanks for contribution @jmealo |
Addresses #101