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

Check if shard names are integers when caching the configurations #312

Merged
merged 1 commit into from
Mar 28, 2023

Conversation

HeyNonster
Copy link
Contributor

@HeyNonster HeyNonster commented Mar 23, 2023

An alternative, or an addition to, #311

We only need to verify that all of the shard names are integers once, otherwise we will perform an expensive integer check on every shard name, every time we call shard_names.

@HeyNonster HeyNonster force-pushed the nony--cache-integer-check branch 3 times, most recently from 83aa878 to 7e8789c Compare March 23, 2023 09:51
@HeyNonster HeyNonster marked this pull request as ready for review March 23, 2023 09:55
@HeyNonster HeyNonster requested review from a team as code owners March 23, 2023 09:55
@HeyNonster HeyNonster force-pushed the nony--cache-integer-check branch from 7e8789c to 6e06d35 Compare March 23, 2023 10:57
@HeyNonster HeyNonster requested review from bquorning and a team March 23, 2023 10:59
We only need to verify that all of the shard names are integers _once_,
otherwise we will perform an expensive integer check on every shard
name, every time we call `shard_names`.
@HeyNonster HeyNonster force-pushed the nony--cache-integer-check branch from 6e06d35 to 1390e38 Compare March 23, 2023 14:46
@HeyNonster HeyNonster changed the title Cache shard name integer check Check if shard names are integers when caching the configurations Mar 23, 2023
@HeyNonster HeyNonster merged commit 14d4f62 into master Mar 28, 2023
@HeyNonster HeyNonster deleted the nony--cache-integer-check branch March 28, 2023 07:58
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