improve(redis): Address various type & usage complaints#3178
improve(redis): Address various type & usage complaints#3178
Conversation
Another step on the road towards noImplicitAny: true. Also address various concurrency/usage issues that Claude has shaken out of the implementation.
pxrl
requested review from
bmzig,
dohaki,
mrice32 and
nicholaspai
as code owners
| return this.decrBy(this.getNamespacedKey(key), 1); | ||
| return this.decrBy(key, 1); |
There was a problem hiding this comment.
decrBy() alsow imposes a namespace on key, so this was causing double-encapsulation.
pxrl
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8577e7b050
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
| try { | ||
| activeInstance = await this.redis.get<string>(this.identifier); | ||
| consecutiveErrors = 0; | ||
| } catch (err) { |
There was a problem hiding this comment.
If redis.get(...) failed then this could potentially hang indefinitely. Permit it to loop up to 10 times before bailing.
| if (!isDefined(redisClients[redisInstanceKey])) { | ||
| // Store the promise immediately so concurrent callers share the same connection attempt. | ||
| redisClients[redisInstanceKey] = _createRedisClient(logger, url, namespace); | ||
| } |
There was a problem hiding this comment.
First assign a promise to avoid a potential race condition where a second redis client is instantiated whilst the first is awaited.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9187ff2274
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b52a32f023
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Another step on the road towards noImplicitAny: true.
Also address various concurrency/usage issues that Claude has shaken out of the implementation.