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

[FFM-9925] - Testgrid - % rollout - Ruby SDK - Fix MM3 hash #33

Merged
merged 6 commits into from
Nov 15, 2023

Conversation

andybharness
Copy link
Contributor

@andybharness andybharness commented Nov 15, 2023

FFM-9925 - Testgrid - % rollout - Ruby SDK - Fix MM3 hash

What
Various fixes to MM3 hash to bring it in alignment with GoLang SDK

  • Fix input to MM3 hash to use <ATTR>:<VALUE> instead of <VALUE>:<ATTR>
  • Set MM3 seed to 0
  • Ignore 0 weighted distributions
  • If bucket_by attribute is missing, fall back to identifier + log new SDK code
  • Additional debug statements for easier diagnostics

Also

  • Remove some IDE files that shouldn't be checked in
  • Bump main version number
  • Loosen Gem dependency requirements

Why
Review SDKs to make sure Murmur3 hash calculations are consistent across SDKs and buckets mappings align for customers that rely on this feature.

Testing
New percentage rollout tests added to testgrid, these will be committed later

erdirowlands and others added 6 commits November 7, 2023 11:33
- Update code to ignore 0 weights
- Additional debug statements around distribution for easier diagnostics
- Remove some Ruby files that shouldn't be checked into source control
Copy link
Contributor

@erdirowlands erdirowlands left a comment

Choose a reason for hiding this comment

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

LGTM, I like that we fall back identifier and the warning, seems logical. I suppose we can get buy in from this with Ethan/Ksenia at the sync next week

@andybharness andybharness merged commit 154cf41 into main Nov 15, 2023
@andybharness andybharness deleted the main-v2 branch November 15, 2023 17:29
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.

2 participants