-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add basic support for redis queue. #1
base: master
Are you sure you want to change the base?
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.
I definitely think a redis client would be awesome. Added some initial comments. Also needs unit tests :)
redis/opts.go
Outdated
|
||
// WithProject configures the Pubsub client with the named project. | ||
func WithRedisAddr(addr string) Option { | ||
return func(opts *Options) { |
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.
we can name WithAddr
since the package name will be used by package consumers redis.WithAddr
redis/opts.go
Outdated
// Option configures the Google Cloud pubsub client. | ||
type Option func(*Options) | ||
|
||
// WithProject configures the Pubsub client with the named project. |
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.
update comment
redis/opts.go
Outdated
} | ||
} | ||
|
||
func WithRedisPassword(password string) Option { |
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.
rename to redis.WithPassword
redis/opts.go
Outdated
} | ||
} | ||
|
||
func WithRedisDB(db int) Option { |
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.
rename to redis.WithDB
redis/opts.go
Outdated
} | ||
} | ||
|
||
func WithRedisQueueName(queueName string) Option { |
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.
rename to redis.WithQueueName
|
||
opts *Options | ||
client *redis.Client | ||
running map[string]*entry |
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.
I think we should store this information in redis so that we can restart the redis server or drone server without losing the state of the queue.
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.
I will try. thanks for reply.
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.
@bradrydzewski is this ok?
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.
Look good to me.
Add basic support for redis queue.