Skip to content

Conversation

@joshuay03
Copy link

@joshuay03 joshuay03 commented Jun 30, 2025

Closes #71.

This is a scope-cut version of buildkite#1 (it included other CI and related changes similar to #72).

@joshuay03 joshuay03 force-pushed the redis-clustering-support branch 2 times, most recently from b0e3b59 to 1a12515 Compare June 30, 2025 05:32
@joshuay03 joshuay03 moved this to In Progress / Pending Review in Open Source Jun 30, 2025
@joshuay03 joshuay03 marked this pull request as ready for review June 30, 2025 05:44
@joshuay03
Copy link
Author

I'm guessing we'd want to add some docs re: the things discussed in the relate issue, I'll tackle that once I've got a review and know that CI is green.

@joshuay03 joshuay03 force-pushed the redis-clustering-support branch 2 times, most recently from bf77d74 to fe9aae5 Compare June 30, 2025 06:01
@joshuay03 joshuay03 force-pushed the redis-clustering-support branch from fe9aae5 to 9651eb2 Compare June 30, 2025 07:06
Comment on lines -33 to +34
if ENV['REDIS_VERSION']
gem 'redis', "~> #{ENV['REDIS_VERSION']}"
if (redis_version = ENV.fetch('REDIS_VERSION', nil))
gem 'redis', "~> #{redis_version}"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason RuboCop was only forcing fetch in some cases below, so I just went ahead and updated all of them.

@justinhoward
Copy link
Member

This is looking really nice, thanks! I haven't yet had time to take a detailed look, but I will try to get that done this week. I'm thinking we should document this as a break of backwards compatibility. The migration path looks to be automatic, but upgrading will cause circuit states to be reset. That's probably not a big issue for most users. Does that make sense to you?

@joshuay03 joshuay03 force-pushed the redis-clustering-support branch from 9651eb2 to cd32bc0 Compare August 23, 2025 23:38
@joshuay03
Copy link
Author

joshuay03 commented Aug 23, 2025

I'm thinking we should document this as a break of backwards compatibility. The migration path looks to be automatic, but upgrading will cause circuit states to be reset. That's probably not a big issue for most users. Does that make sense to you?

Yep, that makes sense since the keys are changing. Is this (CHANGELOG entry?) something you would like me to do? Or will you be handling that?

@justinhoward
Copy link
Member

@joshuay03 Sorry it's taking so long for me to get to this. It's busy season in the education industry. Just wanted you to know I haven't forgotten.

something you would like me to do? Or will you be handling that?

I can handle it as part of the release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error when using with redis-clustering

2 participants