whitfin
Document that keys must implement `String.Chars` when using `Cachex.Router.Ring`
The specs for all of the Cachex functions all specify that the key is an any(), however, when using the Cachex.Router.Ring router module (which seems to be recommended in the latest docs), the key is actually required to be a ExHashRing.Hash.hashable(), which is essentially an alias for String.Chars.t().
Yeah, you're not wrong!
I guess I'm not entirely sure on the best approach here, because routers are third-party pluggable, and I can't control their types. So by that logic Cachex's main interface can't do much beyond any(); is it sufficient to add ExHashRing.Hash.hashable() to the typing on the router implementation itself?
I assume you found this via some type checking utility, would this work for that case?
I actually don't think that type checkers can find this issue - since the router is dynamically dispatched because the module is decided at runtime, then tools like Dialyzer can't know that the type will cause a failure. I ran into this at runtime when testing out my cache, I was receiving a Protocol Not Implemented exception.
I'm not entirely sure if there's a good solution because of the dynamic dispatching thing, but I would definitely update the specs on the implementation and update the implementation's docs, and ideally wherever you recommend the Cachex.Router.Ring router in the docs (it's recommended in a few of the guides) I would add a note explaining the limitation just to ensure that people reading the guides also see the note.
I actually don't think that type checkers can find this issue - since the router is dynamically dispatched because the module is decided at runtime, then tools like Dialyzer can't know that the type will cause a failure. I ran into this at runtime when testing out my cache, I was receiving a Protocol Not Implemented exception.
Could a runtime check raise a more helpful error message than the default Protocol.UndefinedError message?
def check_key(key) do
try do
String.Chars.impl_for!(key)
key
rescue
e in Protocol.UndefinedError ->
raise ArgumentError, "Key must implement String.Chars protocol"
end
end
I agree this should be better documented. I am against runtime errors as above... I think?
I'm wondering if we simply make the requirement that routers only work on binary keys as a flat rule; it's easy enough to convert every other type to some sort of key specifically when using a router.
Of course, that would be a breaking change and we can't do that yet :) so documentation needs a pass in the interim.
I am going to assign this to the 5.x milestone, so that we can handle this with potential breaking changes as necessary. I do think there are improvements we can make to the interfaces as well as the documentation, I'm just unsure what they are yet :)