Resolve redis-v5.0.8 argument type strictness
In gitlab-org/gitlab!134862 (closed), there are many test failures related to incompatible data types being passes as argument to redis
calls. Incompatible data types include: nil
, Date
, Time
, BigDecimal
. The breakages are either due to test stubs being incomplete or existing data-types being handled in redis-v4.8.0 but not in v5.0.8. Note that v5.0.8 is a thin wrapper over the redis-client
gem.
For incomplete test stubs, we can fix the stubs to be more "complete" such that no nil
are passed in. One of such example is to use .perform_inline
to fix worker specs involving JobWaiter
so that a jid
is initialised in the middleware. Without a jid
, JobWaiter.notify
will raise an exception.
Alternatively, we can tighten the logic surrounding the call-site to validate the input, either skipping the call or converting the incompatible data-type into something redis-client
accepts (e.g. calling .to_s
on Date or Time objects).
Remaining broken specs related to data-type
- https://gitlab.com/gitlab-org/gitlab/-/jobs/5396438058
- https://gitlab.com/gitlab-org/gitlab/-/jobs/5396438100
Proposal
We can fix the type compatibility separately before the gem upgrade.
Data types to resolve
-
Date
andTime
types
Occurrences: lib/gitlab/inactive_projects_deletion_warning_tracker.rb
[1] pry(main)> redis = Gitlab::Redis::Queues.with {|c|c}
=> #<Redis client v4.8.0 for unix:///Users/sylvesterchin/work/gitlab-development-kit/redis/redis.socket/1>
[2] pry(main)> redis.hset('testhash', 'a', Date.current, 'b', Date.current.to_s)
=> 2
[3] pry(main)> redis.hset('timehash', 'a', Time.now, 'b', Time.now.to_s)
=> 2
The data written into Redis is identical
redis /Users/sylvesterchin/work/gitlab-development-kit/redis/redis.socket[1]> hgetall testhash
1) "a"
2) "2023-11-15"
3) "b"
4) "2023-11-15"
redis /Users/sylvesterchin/work/gitlab-development-kit/redis/redis.socket[1]> hgetall timehash
1) "a"
2) "2023-11-15 10:46:08 +0800"
3) "b"
4) "2023-11-15 10:46:08 +0800"
-
Boolean types can be converted into a string since they are stored that way in Redis
-
Nil types are handled in redis v4.8 as it converts the
nil
into a''
. However, for redis v5, such conversions must be handled by the client.