Skip to content

Add new flag in CLIENT LIST for import-source client#1398

Merged
zuiderkwast merged 4 commits into
valkey-io:unstablefrom
lyq2333:add-import-flag
Dec 10, 2024
Merged

Add new flag in CLIENT LIST for import-source client#1398
zuiderkwast merged 4 commits into
valkey-io:unstablefrom
lyq2333:add-import-flag

Conversation

@lyq2333

@lyq2333 lyq2333 commented Dec 6, 2024

Copy link
Copy Markdown
Contributor

Fixes #1350 and #1185 (comment).

  • Add new flag "I" in CLIENT LIST for import-source client
  • Add DEBUG_CONFIG for import-mode
  • Allow import-source status to be turned off when import-mode is off

…n import-mode is off

Signed-off-by: lvyanqi.lyq <lvyanqi.lyq@alibaba-inc.com>
Signed-off-by: lvyanqi.lyq <lvyanqi.lyq@alibaba-inc.com>
@codecov

codecov Bot commented Dec 6, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.87%. Comparing base (6df376d) to head (83792d0).
Report is 11 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #1398      +/-   ##
============================================
+ Coverage     70.82%   70.87%   +0.04%     
============================================
  Files           118      118              
  Lines         63561    63592      +31     
============================================
+ Hits          45020    45071      +51     
+ Misses        18541    18521      -20     
Files with missing lines Coverage Δ
src/config.c 77.63% <ø> (ø)
src/networking.c 88.61% <100.00%> (+0.08%) ⬆️

... and 14 files with indirect coverage changes

@zuiderkwast zuiderkwast left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in general. Thanks!

Comment thread src/networking.c Outdated
Comment thread src/config.c
Comment thread tests/unit/expire.tcl Outdated
@zuiderkwast

zuiderkwast commented Dec 6, 2024

Copy link
Copy Markdown
Contributor

Should I add a new option for CLIENT KILL, i.e. CLIENT KILL FLAG?

It's a good idea, but let's move this idea to #668.

Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Yanqi Lv <lvyanqi.lyq@alibaba-inc.com>
@lyq2333

lyq2333 commented Dec 9, 2024

Copy link
Copy Markdown
Contributor Author

Should I add a new option for CLIENT KILL, i.e. CLIENT KILL FLAG?

It's a good idea, but let's move this idea to #668.

Thanks for your review and suggestion. I like this idea 😄

@zuiderkwast zuiderkwast left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@valkey-io/core-team please approve the interface changes mentioned in the top comment.

@zuiderkwast zuiderkwast added release-notes This issue should get a line item in the release notes major-decision-pending Major decision pending by TSC team needs-doc-pr This change needs to update a documentation page. Remove label once doc PR is open. labels Dec 9, 2024
Comment thread tests/unit/expire.tcl
Co-authored-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Yanqi Lv <lvyanqi.lyq@alibaba-inc.com>
@madolson madolson added major-decision-approved Major decision approved by TSC team and removed major-decision-pending Major decision pending by TSC team labels Dec 10, 2024
@zuiderkwast zuiderkwast merged commit f951a1c into valkey-io:unstable Dec 10, 2024
@zuiderkwast

Copy link
Copy Markdown
Contributor

@lyq2333 Can you open a doc PR to add the flag here: https://github.com/valkey-io/valkey-doc/blob/main/commands/client-list.md

@lyq2333

lyq2333 commented Dec 11, 2024

Copy link
Copy Markdown
Contributor Author

@lyq2333 Can you open a doc PR to add the flag here: https://github.com/valkey-io/valkey-doc/blob/main/commands/client-list.md

OK, I will include it in the introduction of import mode.

vudiep411 pushed a commit to Autxmaton/valkey that referenced this pull request Dec 15, 2024
- Add new flag "I" in `CLIENT LIST` for import-source client
- Add `DEBUG_CONFIG` for import-mode
- Allow import-source status to be turned off when import-mode is off

Fixes valkey-io#1350 and
valkey-io#1185 (comment).

---------

Signed-off-by: lvyanqi.lyq <lvyanqi.lyq@alibaba-inc.com>
Signed-off-by: Yanqi Lv <lvyanqi.lyq@alibaba-inc.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Binbin <binloveplay1314@qq.com>
zuiderkwast added a commit to valkey-io/valkey-doc that referenced this pull request Jan 2, 2025
This PR is for valkey-io/valkey#1185 and
valkey-io/valkey#1398.

---------

Signed-off-by: lvyanqi.lyq <lvyanqi.lyq@alibaba-inc.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

major-decision-approved Major decision approved by TSC team needs-doc-pr This change needs to update a documentation page. Remove label once doc PR is open. release-notes This issue should get a line item in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fail CLIENT IMPORT-SOURCE When Import Mode is Disabled and Reflect Client Import Source State in CLIENT LIST

5 participants