search

hibiken / asynq

Simple, reliable, and efficient distributed task queue in Go
MIT License
10k stars 714 forks source link

[BUG] rate.NewSemaphore: type conversion failure (MakeRedisClient error?) #790

Closed gwkline closed 11 months ago

gwkline commented 11 months ago

Describe the bug When trying to create a new semaphore using the rate package, I am getting the following error:

rate.NewSemaphore: unsupported RedisConnOpt type asynq.RedisClientOpt

I believe this is happening because calling asynq.MakeRedisClient() on a asynq.RedisClientOpt is returning a *redis.Client which doesn't implement all methods required by redis.UniversalClient. Maybe this is because of a mismatch between the go-redis version? I'm not quite sure.

To Reproduce

package scripts

import (
    "context"
    "fmt"
    "os"

    "my-app/pkg/task"
    "github.com/hibiken/asynq"
    "github.com/hibiken/asynq/x/rate"

    "github.com/redis/go-redis/v9" // also fails on v8
)

func (s *Service) SomeFunc(ctx context.Context) error {
    ops, err := redis.ParseURL(os.Getenv("REDIS_URL"))
    if err != nil {
        fmt.Printf("failed parsing redis: %v", err)
    }

    asynqOps := asynq.RedisClientOpt{
        Addr:     ops.Addr,
        DB:       ops.DB,
        Password: ops.Password,
        Username: ops.Username,
    }

    //....

         semaphore := rate.NewSemaphore(asynqOps, "someKey", 50) //error is happening in here**

    //....
}

Expected behavior When passing a valid asynq.RedisClientOpt to rate.NewSemaphore, there should not be a type coercion error.

Environment (please complete the following information):

kamikazechaser commented 11 months ago

Looking at your environment, The x package is not compatible with v0.24.x at the moment. I can see we reverted it back to go-redis/v8. Could you try running your snippet when compiled with an older version of github.com/hibiken/asynq?

kamikazechaser commented 11 months ago

Could you test https://github.com/hibiken/asynq/pull/796 and let me know if it fixes this issue.

cc/ @amaury1729

gwkline commented 11 months ago

@kamikazechaser #796 does appear to solve this issue!