-
Notifications
You must be signed in to change notification settings - Fork 4
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
Handle units in some YAML settings (post CASSANDRA-15234) #10
Conversation
@@ -31,7 +31,9 @@ data class CassandraYaml(val data : JsonNode) : YamlConfig(data) { | |||
val partitioner get() = get("partitioner") | |||
val memtableAllocationType get() = get("memtable_allocation_type") | |||
val authorizer get() = get("authorizer") | |||
private val rowCacheSizeInMB get() = (get("row_cache_size","0")).toInt() | |||
val rowCacheSizeInMB get() = ( | |||
get("row_cache_size", "0").replace("MiB", "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be GiB (or KiB), or GB, MB, KB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could I then delete all non-numeric characters ? Something like .replace("\D+", "")
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, i agree to that
(since we're only looking for non-zero here)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed a commit doing this.
It seems that at least due to CASSANDRA-15234 the
cassandra.yaml
file started to feature units (egMiB
) for some settings.Montecristo is mostly not troubled by this, except for the
row_cache_size
where we actually parse the value to see if it's bigger than 0.This PR improves that parsing by removing the
MiB
string. I could not find a decent(-ly simple) de-humanization library to drop in, so I went for a quick fix like this.I'm also having issues with the unit tests. I think the
Assertions
library is not taken into the accunt. I'm commiting a failing unit test, hoping to see the CI here to fail.