-
Notifications
You must be signed in to change notification settings - Fork 54
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
Adding cache client to nodes manager #609
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This PR updates the nodes manager initialization to include a cache client parameter, enabling the consumption of Redis-cached data.
- Adds a cache parameter to the CreateNodes function in pkg/nodes/nodes.go.
- Updates calls to CreateNodes in various commands and tests.
- In some command files, passes an incorrect type instead of a cache client instance.
Reviewed Changes
File | Description |
---|---|
pkg/nodes/nodes.go | Adds a new parameter for cache client in the function signature. |
cmd/cli/main.go | Updates CreateNodes call by passing nil for the cache client. |
pkg/queries/queries_test.go | Updates CreateNodes call in tests to pass nil for the cache client. |
cmd/admin/main.go | Incorrectly passes redis.Client type instead of an instance. |
cmd/tls/main.go | Incorrectly passes redis.Client type instead of an instance. |
cmd/api/main.go | Incorrectly passes redis.Client type instead of an instance. |
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
@@ -683,7 +683,7 @@ func osctrlAdminService() { | |||
log.Info().Msg("Initialize settings") | |||
settingsmgr = settings.NewSettings(db.Conn) | |||
log.Info().Msg("Initialize nodes") | |||
nodesmgr = nodes.CreateNodes(db.Conn) | |||
nodesmgr = nodes.CreateNodes(db.Conn, redis.Client) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing redis.Client as a parameter is incorrect; you should pass an instance of redis.Client (e.g., using redis.NewClient(...)).
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
@@ -628,7 +628,7 @@ func osctrlService() { | |||
log.Info().Msg("Initialize settings") | |||
settingsmgr = settings.NewSettings(db.Conn) | |||
log.Info().Msg("Initialize nodes") | |||
nodesmgr = nodes.CreateNodes(db.Conn) | |||
nodesmgr = nodes.CreateNodes(db.Conn, redis.Client) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing redis.Client as a parameter is incorrect; ensure you pass a valid instance of redis.Client via an appropriate constructor.
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
@@ -489,7 +489,7 @@ func osctrlAPIService() { | |||
log.Info().Msg("Initialize settings") | |||
settingsmgr = settings.NewSettings(db.Conn) | |||
log.Info().Msg("Initialize nodes") | |||
nodesmgr = nodes.CreateNodes(db.Conn) | |||
nodesmgr = nodes.CreateNodes(db.Conn, redis.Client) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The call uses redis.Client incorrectly instead of a cache client instance; please change it to pass an instance created by redis.NewClient(...).
nodesmgr = nodes.CreateNodes(db.Conn, redis.Client) | |
nodesmgr = nodes.CreateNodes(db.Conn, redisClient) |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
Adding cache client to nodes manager so data can be consumed from the cache (Redis).