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

feat!: migrate to go-redis as redis client #334

Merged
merged 3 commits into from
Mar 17, 2022

Conversation

hongalex
Copy link
Collaborator

@hongalex hongalex commented Mar 17, 2022

What type of PR is this?
/kind refactor

What this PR does / Why we need it:
Migrates current client from redigo to go-redis

BREAKING: redigo and go-redis options are NOT compatible. The following options have been modified.

DROPPED

redis_pool_maxIdle
redis_pool_wait

CHANGED

redis_pool_minActive -> redis_pool_size
redis_pool_idleTimeout -> redis_idle_timeout

ADDED

redis_min_idle_conns
redis_max_retries
redis_min_retry_backoff
redis_max_retry_backoff

Which issue(s) this PR fixes:
Closes #333

Special notes for your reviewer:

@hongalex hongalex requested review from lorangf and yuryu March 17, 2022 18:56
Copy link
Member

@yuryu yuryu left a comment

Choose a reason for hiding this comment

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

I think it should be feat! instead refactor! (I might've done this in the past but refactoring shouldn't change the behavior, if it does it's not refactoring).

otherwise lgtm

@yuryu yuryu changed the title refactor!: migrate to go-redis as redis client feat!: migrate to go-redis as redis client Mar 17, 2022
@yuryu
Copy link
Member

yuryu commented Mar 17, 2022

Also this breaking change is fine because we didn't release the previous behavior.

@hongalex hongalex merged commit b91a5f2 into googleforgames:main Mar 17, 2022
@hongalex hongalex deleted the refactor-redis branch March 17, 2022 19:45
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.

Change redigo to go-redis
2 participants