scylladb / scylla-ccm

Cassandra Cluster Manager, modified for Scylla
Apache License 2.0
20 stars 60 forks source link

support multiple identical args in SCYLLA_EXT_OPTS #558

Closed fruch closed 3 months ago

fruch commented 5 months ago

since we have cases multiple --experimental-features needs to be passed via SCYLLA_EXT_OPTS, adding support for key with multiple values in process_opts()

fixes: #540 closes: #541

Testing - to be on the safe side:

tchaikov commented 5 months ago

might want to reference #541.

fruch commented 5 months ago

might want to reference #541.

yeah, I forgot @eliransin was trying to solved it in #541 now that I look at it, it's similar but a bit longer than this one

fruch commented 5 months ago

Looks good to me. I assumed you tested it actually works? :-)

it works for the cases reported, i.e. multiple --experimental-features

fruch commented 5 months ago

tests showed, that a few places were needed to change and expect a list, not assume an int.

fixed those and re-running gating

fruch commented 3 months ago

Code is failing in gating

Need to check why it's still breaking Scylla boot

bhalevy commented 3 months ago

@nyh by the way, why don't we support comma-separated list arg in scylla --experimental-features?

nyh commented 3 months ago

I don't know (I don't think I wrote that code?), we definitely could. But we need to also keep supporting the current approach (multiple instances of the option), because it's convenient in scripts.

fruch commented 3 months ago

Code is failing in gating

Need to check why it's still breaking Scylla boot

found the issue, the args without value, was passing empty space as value (it wasn't clear from logs) now fixe, and giving one more go in gating