Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support for CLIENT KILL MAXAGE #2727

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

atakavci
Copy link
Contributor

Closes/Fixes #2726

added two overloads and use a filter object to avoid a breaking change.

@NickCraver
Copy link
Collaborator

Reviewed on the call today - in general, this pattern won't work due to the server-side API supporting n filters, we'd need to allow n of them ordered on the command. We'd either need to take a params here or more likely do a builder pattern for n filters. In a builder, returning itself, we could do things like making a method per filter that's very specific and only has the args needed for the 7 doced paths:

  • CLIENT KILL ADDR ip:port
  • CLIENT KILL LADDR ip:port
  • CLIENT KILL ID client-id
  • CLIENT KILL TYPE type
  • CLIENT KILL USER username
  • CLIENT KILL SKIPME yes/no
  • CLIENT KILL MAXAGE maxage

This wouldn't be very dissimilar to the constructors you have now in terms of one-per, but we can make it intuitive and pit-of-success here for users to do the right thing. And we can make it write the message directly on the backend since it's in the base library. Picture for example:

IServer.ClientKillAsync(new ClientKillFilter().WithAddr(...).WithSkipMe(true));`

Thoughts?

@atakavci
Copy link
Contributor Author

hey @NickCraver , sounds nice and clear ! please take a look with new commit.

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.

Support MAXAGE option for CLIENT KILL
2 participants