Add support for autotune limits in service_shard_update #3830
Conversation
| parser.add_argument( | ||
| "-d", | ||
| "--dry-run", | ||
| help="Do not commit changes to git", | ||
| action="store_true", | ||
| dest="dry_run", | ||
| ) |
There was a problem hiding this comment.
++
(i'm also a fan of --for-real flags so that dry-run is the default, but that would be a little more annoying to rollout here)
|
@nemacysts sorry for the 6 months of suspense! kinda forgot about this PR until we ran into some super anemic ssr shards with 0.1 cpus causing hella timeouts on stage lol |
| for resource in list(limit_config): | ||
| for key in list(limit_config[resource]): |
There was a problem hiding this comment.
(these list()'s aren't necessary, but i'm assuming they're were helpful in some way while you were debugging and converting from a view to a list shouldn't really impact perf in a meaningful way for this script)
There was a problem hiding this comment.
unfortunately they are necessary under py3 :( without them i get this
Traceback (most recent call last):
File "paasta_tools/contrib/service_shard_update.py", line 350, in <module>
main(args)
File "paasta_tools/contrib/service_shard_update.py", line 297, in main
for key in limit_config[resource]:
RuntimeError: dictionary changed size during iteration
There was a problem hiding this comment.
oh d'oh - i'm dumb: we're mutating the dict
i blame friday-brain :p
|
yep, got the expected output :) will merge this next week |
Added to PaaSTA at large in #3744. Also pulls in commit from #3829 to make min/max instance values optional and adds a new
--dry-runparam that skips committing and pushing changes to yelpsoa-configs.Confirmed that the new kwargs do as expected, generating valid configs. If anyone knows a way to do this that doesn't involve a mess of nested if statements, I'm all ears lol