Skip to content

explicitly allow '*' in specs for clique_usage:register/2#3

Merged
martinsumner merged 1 commit intoopenriak-3.2from
tiot/openriak-3.2/allow-asterisk-atoms-in-clique_usage
Nov 25, 2025
Merged

explicitly allow '*' in specs for clique_usage:register/2#3
martinsumner merged 1 commit intoopenriak-3.2from
tiot/openriak-3.2/allow-asterisk-atoms-in-clique_usage

Conversation

@hmmr
Copy link
Contributor

@hmmr hmmr commented Nov 7, 2025

else, dialyzer will complain about specs like this:

clique:register_usage(["riak-admin", "tictacaae", "rebuild-schedule", '*'], ...)

else, dialyzer will complain about specs like this:

clique:register_usage(["riak-admin", "tictacaae", "rebuild-schedule",
                      '*'], ...)
@martinsumner
Copy link
Contributor

martinsumner commented Nov 25, 2025

Can I just check I understand this correctly.

Is there a need to give consistent usage advice regardless what part of the input is:

i.e.

if someone inputs:

riak admin tictacaae storeheads node or
riak admin tictacaae storeheads partition the storeheads_usage() usage should be returned.

This isn't relying on any clever matching rules in ets - it will match because the code will explicitly lookup '*'.

Dialyzer complains because '*' is an atom(). Though $* could have been used - and there would be no dialyzer issue, or is there something I'm missing? Is there a reason for this to be * as an atom() not * as a string()?

@hmmr
Copy link
Contributor Author

hmmr commented Nov 25, 2025

The choice of '*' as an atom is original authors'. Specs with '*' already exist in riak_core, e.g. https://github.com/OpenRiak/riak_core/blob/openriak-3.2/src/riak_core_cluster_cli.erl#L292, meaning changing it to a '$' (or "*") would snowball into a bunch of new PRs (and affect users of clique outside of riak).

I note that the issue dialyzer is taking with the spec is perfectly valid. Exactly why dialyzer in previous OTP versions didn't flag this case, is an interesting question (it is this issue being discussed here, I think).

if someone inputs:

riak admin tictacaae storeheads node or
riak admin tictacaae storeheads partition the storeheads_usage() usage should be >returned.

Without -n node or -p partition, the command assumes current node and -p all. If, with or without those options, a boolean value is given, it is set for the node/partition(s) specified or inferred. If an argument is present but cannot be converted to a boolean, usage is printed (however, this is a matter to discuss in OpenRiak/riak_kv#31).

@martinsumner martinsumner self-requested a review November 25, 2025 17:15
@martinsumner martinsumner merged commit 76de285 into openriak-3.2 Nov 25, 2025
2 checks passed
@hmmr hmmr deleted the tiot/openriak-3.2/allow-asterisk-atoms-in-clique_usage branch November 25, 2025 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants