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.

Sign up for GitHub

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

Jump to bottom

[ADDED] Ability to express the max_file_store and max_memory_store as a string (100M, etc..) #2777

Merged
merged 5 commits into from Jan 13, 2022
Merged

[ADDED] Ability to express the max_file_store and max_memory_store as a string (100M, etc..) #2777

merged 5 commits into from Jan 13, 2022

Conversation

hooksie1
Copy link
Contributor

Resolves #2754

Changes proposed in this pull request:

  • Adds a function to check if max_file_store is a string or int64. Possibly could be reused again.

/cc @nats-io/core

@hooksie1
Check if value is string or int64
d10c306
@hooksie1
Add parsing of string for sizes
12f8179 
Checks the suffix of the string to create the size of the int64
kozlovic
kozlovic requested changes Jan 12, 2022
View reviewed changes
server/opts.go Outdated Show resolved Hide resolved
server/opts.go Outdated Show resolved Hide resolved
server/opts.go Outdated Show resolved Hide resolved
server/opts.go Outdated Show resolved Hide resolved
server/opts_test.go Show resolved Hide resolved
server/opts.go Outdated Show resolved Hide resolved
@hooksie1
Review
d956d6d 
Rename function
More easily read math
merged functions together
Changed from predefining error
Fix empty string issue
use same function for max mem store
@hooksie1
Fix non used error
4aa6c62
kozlovic
kozlovic requested changes Jan 12, 2022
View reviewed changes
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

Sorry for the requests to change again, but I did not notice it during the first review.

server/opts.go Outdated Show resolved Hide resolved
@hooksie1
Remove switch
0b77d51 
Removes switch statement and uses map for finding correct multiple

Also uses ParseInt instead of Atoi for getting string integer.
@hooksie1
Copy link
Contributor Author

No problem at all, that's much cleaner than what I had initially. Thanks for the really helpful input!

@kozlovic kozlovic changed the title Check if value is string or int64 [ADDED] Ability to express the max_file_store and max_memory_store as a string (100M, etc..) Jan 13, 2022
kozlovic
kozlovic approved these changes Jan 13, 2022
View reviewed changes
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

LGTM

@kozlovic
Copy link
Member

@hooksie1 Thank you for the contribution!

@kozlovic kozlovic merged commit a7554bd into nats-io:main Jan 13, 2022
@kozlovic kozlovic mentioned this pull request Jan 21, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kozlovic kozlovic kozlovic approved these changes

@wallyqs wallyqs Awaiting requested review from wallyqs

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The "max_file_store" option cannot be quoted
2 participants
@hooksie1 @kozlovic