Skip to content
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 a few warnings to the connection switcher and conf parser #143

Merged
merged 1 commit into from
Oct 25, 2017

Conversation

bquorning
Copy link
Member

Mostly extracted from #136.

@bquorning bquorning requested review from jacobat and grosser October 13, 2017 14:16
@grosser
Copy link
Contributor

grosser commented Oct 13, 2017

... better double-check shards in kubernetes are integers first... I'll update here once I checked

@grosser
Copy link
Contributor

grosser commented Oct 13, 2017

kk, all good!

@bquorning bquorning changed the base branch from master to v4 October 25, 2017 09:25
@bquorning bquorning force-pushed the bquorning/warnings branch 2 times, most recently from 0079c79 to 2a1721d Compare October 25, 2017 09:33
@bquorning bquorning closed this Oct 25, 2017
@bquorning bquorning reopened this Oct 25, 2017
@bquorning bquorning merged commit 43ce9ea into v4 Oct 25, 2017
@bquorning bquorning deleted the bquorning/warnings branch October 25, 2017 09:58
bquorning added a commit that referenced this pull request Oct 25, 2017
Add a few warnings to the connection switcher and conf parser
bquorning added a commit that referenced this pull request Oct 25, 2017
Add a few warnings to the connection switcher and conf parser
@bquorning bquorning mentioned this pull request Oct 25, 2017
@codebender
Copy link

codebender commented May 23, 2019

This was a breaking change to the gem for everyone using strings to name the shards.

@grosser
Copy link
Contributor

grosser commented May 24, 2019

I think this was about sanitizing inputs ... I guess casting to int would have worked too.
Who was using strings as shards ? ... still need that ?

@codebender
Copy link

Hi @grosser,

Having named shards is really handy when you want to read/write to a specific shard that is feature specific and not "account" ID specific like in the readme.

It looks like these other people also prefer string names: #195

@grosser
Copy link
Contributor

grosser commented May 24, 2019

yeah sounds reasonable, can you make a PR that semi-reverts this one and allows strings ?
(change the tests to test with a mix of strings and ints)
/cc @bquorning

@holmes2
Copy link

holmes2 commented Aug 17, 2020

@bquorning @grosser
Was there a reason why only integers are allowed for shard_names ?
I am trying to upgrade rails service to a higher version of this gem but still because of the shard name restriction we are not able to do it.
Is there a way to get around this shard naming restriction ?

@grosser
Copy link
Contributor

grosser commented Aug 17, 2020

I think this is because we only use it with integers and we wanted to warn our users from misconfiguration

@bquorning bquorning added this to the v4 milestone Dec 20, 2021
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.

4 participants