misc(cli): use scallop instead of sumac #854
Conversation
szymonm
changed the title
|
You might need to rebase from the develop branch for the build to pass. |
|
Fixed, @broneill |
|
Are there any concerns? Are the breaking changes OK? I can do some trickery to keep backwards compatibility, but that will be less elegant. Also, I see that some docs around this will need to be changed. |
|
I'd like to get some feedback from @velvia . |
|
I’ll have a look soon, like today or over the weekend…….
… On Aug 14, 2020, at 6:16 AM, broneill ***@***.***> wrote:
I'd like to get some feedback from @velvia <https://github.com/velvia> .
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#854 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIDPW7KEVHNILBIPYTTTR3SAU2MNANCNFSM4P3PLPEQ>.
|
|
Pinging @velvia |
There was a problem hiding this comment.
@szymonm thanks, this is a great change. Sumac development has stopped and scallop has much more support, and this will give us much more flexibility to improve the verbosity of the CLI going forward.
Since the options aren't changing I don't think any changes to README are needed...
| require(args.host.isDefined && args.dataset.isDefined && args.labelNames.isDefined, "--host, --dataset and --labelName must be defined") | ||
| val remote = Client.standaloneClient(system, args.host(), args.port()) | ||
| val options = QOptions(args.limit(), args.sampleLimit(), args.everyNSeconds.map(_.toInt).toOption, | ||
| timeout, args.shards.map(_.map(_.toInt)).toOption, args.spread.toOption.map(Integer.valueOf)) |
There was a problem hiding this comment.
Is Integer.valueOf for args.spread necessary? Spread is declared with opt[Int]....
There was a problem hiding this comment.
QOptions constructor takes Option[Integer] (don't know the reason, seems like unfounded inconsistency).
Pull Request checklist
Current behavior :
Part of #457
quantifind.sumacis not being ported to Scala 2.12. Instead, I migrateCliMainto use scallopthat has already been used in
gateway.BREAKING CHANGES
Scallop uses different way of parsing lists and maps.
Specifically, uses space (
=) for value definition in map.This applies to following parameters of
CliMain:labelNames: instead of--labelNames a,b,cprogram expects--labelNames a b c.shards: instead of--shards a,b,cprogram expects--shards a b c.labelFilter: instead of--labelFilter key1:value1,key2:value2it expects--labelFilter key1=value1 key2=value2.