Workhorse: remove the global Redis client
What does this MR do and why?
While running tests locally for the first time, I got a unit test panic:
=== RUN TestKeyChangesInstantReturn
=== RUN TestKeyChangesInstantReturn/sees_change_with_key_existing_and_changed
--- FAIL: TestKeyChangesInstantReturn (0.00s)
--- FAIL: TestKeyChangesInstantReturn/sees_change_with_key_existing_and_changed (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x8 pc=0x1054b0cc0]
goroutine 13 [running]:
testing.tRunner.func1.2({0x105761a80, 0x106029ed0})
/opt/homebrew/opt/go/libexec/src/testing/testing.go:1545 +0x1c8
testing.tRunner.func1()
/opt/homebrew/opt/go/libexec/src/testing/testing.go:1548 +0x360
panic({0x105761a80?, 0x106029ed0?})
/opt/homebrew/opt/go/libexec/src/runtime/panic.go:914 +0x218
gitlab.com/gitlab-org/gitlab/workhorse/internal/redis.TestKeyChangesInstantReturn.func1(0x140000be701?)
/Users/mike/src/gitlab/workhorse/internal/redis/keywatcher_test.go:118 +0x50
testing.tRunner(0x140005f3040, 0x140006079a0)
/opt/homebrew/opt/go/libexec/src/testing/testing.go:1595 +0xe8
created by testing.(*T).Run in goroutine 12
/opt/homebrew/opt/go/libexec/src/testing/testing.go:1648 +0x33c
So, I decided to fix it by checking errors properly in initRdb()
and cleanly reporting them. Got this, which is much better than a panic:
=== RUN TestKeyChangesInstantReturn
keywatcher_test.go:23:
Error Trace: /Users/mike/src/gitlab/workhorse/internal/redis/keywatcher_test.go:23
/Users/mike/src/gitlab/workhorse/internal/redis/keywatcher_test.go:79
Error: Received unexpected error:
open ../../config.toml: no such file or directory
Test: TestKeyChangesInstantReturn
--- FAIL: TestKeyChangesInstantReturn (0.00s)
Then I decided to have a more holistic look at the code as that package-level field looks fishy. I think it's much nicer without it.
Screenshots or screen recordings
N/A
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Edited by Mikhail Mazurskiy