redis rework base

[kwitsch]

• rewrote old redis logic with rueidis
• removed almost all redis unit tests(they rely on minredis which isn't compatible with rueidis)

As most of the logic will change during implementation of #945 unit tests should be added afterwards.

[ThinkChaos]

Looks like a lot of the changes are from auto formatting. Did you use gofumpt?
Running gofumpt -w ./**.go doesn't make any changes for me.
I think that would be nice as a separate MR/commit.

I did take a quick look at the Redis changes, and only thing that stuck out to me was storing the context in Client. I think it's best practice to avoid that and have a context argument for every method.

[kwitsch]

I did take a quick look at the Redis changes, and only thing that stuck out to me was storing the context in Client. I think it's best practice to avoid that and have a context argument for every method.

That was intended to be used later on. Had the idea to use a cancellation context which is stored inside the client and is used for all requests. Then we can implement a client close method that cancels the context so all requests are terminated gracefully.

Sorry I'm not that familiar with context best practices. 😓

Is it better to use context.Background() at every call instead of doing it one time and storing it?

[kwitsch]

Looks like a lot of the changes are from auto formatting. Did you use gofumpt?
Running gofumpt -w ./**.go doesn't make any changes for me.

I used make fmt because make lint wouldn't run.
Wasn't that the correct fix?
Often did that in the past.
Do we need to modify the Makefile?

[ThinkChaos]

Wasn't that the correct fix?

Yeah that's what I'd do too.
What I don't understand is when I run make fmt on main (ea95d36) I don't get any of those changes.

That was intended to be used later on. Had the idea to use a cancellation context which is stored inside the client and is used for all requests. Then we can implement a client close method that cancels the context so all requests are terminated gracefully.

In that case I think we can use rueidis' Client.Close.
But yeah the recommended use of a context is for the caller to pass it with each call/action so that each one can have an independent timeout/cancellation.
See Contexts and structs for more details.

[kwitsch]

Thanks for the info.
I'll change it.

Regarding the fmt problem: I'll try to reproduce it to see what went wrong

[kwitsch]

Regarding the fmt problem: I'll try to reproduce it to see what went wrong

I currently can't reproduce it and also don't remember anything else I did at this commit. 🫤

I'll close this PR and implement the context changes on a new branch.

[0xERR0R]

I though about Redis integration, I think it would make sense to create a new branch fb-redis-integration for long running redis implementation tasks. And put all related PRs on this branch. We would have in this case main without CI buld errors and we can use this branch to integrate and test redis. Afterwards we can merge it into master. @kwitsch what do you think?

[kwitsch]

I though about Redis integration, I think it would make sense to create a new branch fb-redis-integration for long running redis implementation tasks. And put all related PRs on this branch. We would have in this case main without CI buld errors and we can use this branch to integrate and test redis. Afterwards we can merge it into master. @kwitsch what do you think?

Sounds good.
@0xERR0R if you create the branch on the main repository I'll request future PRs to it.

[0xERR0R]

Branch was created