Make METADATA LIST use ERR_NOMATCHINGKEYS when no keys to return

[DanielOaks]

This just makes METADATA LIST return an ERR_NOMATCHINGKEYS event when there are no keys to return, instead of just returning the RPL_METADATAEND, following a conversation with Aerdan.

[attilamolnar]

What's behind the change?

[DanielOaks]

Log from #ircv3

< danneh_> re Metadata Specification, with METADATA LIST, it says:   The response will be one or more RPL_KEYVALUE events and a closing RPL_METADATAEND event.
< danneh_> if there are zero metadata keys to return, should we return zero RPL_KEYVALUE events and just the closing RPL_METADATAEND
< jwheare> 0 or more makes more sense to me
< danneh_> o i c, DarthGandalf rewrote it to be this a few days ago, silly me not pulling latest changes:   The response will be zero or more `RPL_KEYVALUE` events, following by `RPL_METADATAEND` event.
< Aerdan> dunno why, ERR_NOMATCHINGKEYS is there for a reason.
< danneh_> ERR_NOMATCHINGKEYS is used in METADATA GET when the client queries for specific keys, and the key they request doesn't exist
< danneh_> i couldn't see that being used for METADATA LIST because there's not really anything we're matching keys to, doesn't make sense for the message "no matching keys"
< danneh_> if we wanted to return something like that for LIST we'd probably need to have an ERR_NOKEYS instead?
< danneh_> i dunno, we could take it as we're matching keys to the target in LIST and return ERR_NOMATCHINGKEYS but that seems ugly, easier to just return no results imo
< Aerdan> you're asking for a set of keys that don't exist. ERR_NOMATCHINGKEYS works just fine.
< danneh_> fair enough, will do that then
< danneh_> hmm, in that case, instead of:     `<Key> :no matching keys`    ERR_NOMATCHINGKEYS would return:   `<Target> :no matching keys`   , or just ignore <Key> and return the line?
< Aerdan> * is an acceptable entry.
< danneh_> ah 'kay, thanks

[DarthGandalf]

So what's wrong with sending RPL_METADATAEND without any RPL_KEYVALUE to
mark that no keys exist?

2015-03-25 20:58 GMT+00:00 Daniel Oaks notifications@github.com:

Log from #ircv3

< danneh_> re Metadata Specification, with METADATA LIST, it says: The response will be one or more RPL_KEYVALUE events and a closing RPL_METADATAEND event.
< danneh_> if there are zero metadata keys to return, should we return zero RPL_KEYVALUE events and just the closing RPL_METADATAEND
< jwheare> 0 or more makes more sense to me
< danneh_> o i c, DarthGandalf rewrote it to be this a few days ago, silly me not pulling latest changes: The response will be zero or more RPL_KEYVALUE events, following by RPL_METADATAEND event.
< Aerdan> dunno why, ERR_NOMATCHINGKEYS is there for a reason.
< danneh_> ERR_NOMATCHINGKEYS is used in METADATA GET when the client queries for specific keys, and the key they request doesn't exist
< danneh_> i couldn't see that being used for METADATA LIST because there's not really anything we're matching keys to, doesn't make sense for the message "no matching keys"
< danneh_> if we wanted to return something like that for LIST we'd probably need to have an ERR_NOKEYS instead?
< danneh_> i dunno, we could take it as we're matching keys to the target in LIST and return ERR_NOMATCHINGKEYS but that seems ugly, easier to just return no results imo
< Aerdan> you're asking for a set of keys that don't exist. ERR_NOMATCHINGKEYS works just fine.
< danneh_> fair enough, will do that then
< danneh_> hmm, in that case, instead of: <Key> :no matching keys ERR_NOMATCHINGKEYS would return: <Target> :no matching keys , or just ignore and return the line?
< Aerdan> * is an acceptable entry.
< danneh_> ah 'kay, thanks

—
Reply to this email directly or view it on GitHub
#129 (comment)
.

[attilamolnar]

NAK, I prefer the current version.

[attilamolnar]

It's simpler to handle the current version because clients can add the metadata replies to a list, and check the contents of the list whenever the end of list numeric arrives. With this change they'd have to handle both the end of list numeric and the ERR_ numeric as well, which is more complexity for zero gain.

[ghost]

In that case, remove ERR_NOMATCHINGKEYS entirely...?

[DanielOaks]

ERR_NOMATCHINGKEYS is also used in METADATA GET to show that a requested key could not be retrieved, could maybe modify it to ERR_NOMATCHINGKEY or something, but we'd need to leave one of those in for that subcommand unless we rework it.

[attilamolnar]

In that case, remove ERR_NOMATCHINGKEYS entirely...?

I don't see a reason to do that since ERR_NOMATCHINGKEYS makes sense the way how METADATA GET works: it sends no end of list numeric. If ERR_NOMATCHINGKEYS is removed, METADATA GET must be rewritten to 1. always send an end of list numeric 2. send no numeric for a key that does not exist.

[DarthGandalf]

could maybe modify it to ERR_NOMATCHINGKEY or something

I agree

[DanielOaks]

Alright, I should be fine to close this then?

[attilamolnar]

Yes, please continue the discussion regarding the removal or modification of ERR_NOMATCHINKGEYS in #130.