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

Batching #73

Merged
merged 11 commits into from
Mar 25, 2024
Merged

Batching #73

merged 11 commits into from
Mar 25, 2024

Conversation

gbrindisi
Copy link
Contributor

@gbrindisi gbrindisi commented Mar 15, 2024

Pull Request

This pull request implement a configuration to enable batching the maximum amount of eligible repositories in scope for every Evergreen run.

Proposed Changes

Add a config variable that is compared to the eligible_repositories counter and breaks the repositories loop when the batch value is met.

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request
  • run make lint and fix any issues that you have introduced
  • run make test and ensure you have test coverage for the lines you are introducing

Reviewer

  • Label as either bug, documentation, enhancement, infrastructure, or breaking

@gbrindisi gbrindisi requested a review from zkoppert as a code owner March 15, 2024 10:39
@zkoppert zkoppert added the enhancement New feature or request label Mar 19, 2024
@zkoppert
Copy link
Member

Hey @gbrindisi ! Thanks for opening up this pull request. Can you help me understand the use case for limiting the number of eligible repos to a max count? Is this for making sure the workflow doesn't take too long?

@gbrindisi
Copy link
Contributor Author

@zkoppert hi! when rolling out changes to a large sample of targets (such as rolling out dependabot), we might not always have the confidence to fully assess the impact from the start.

In cases like this it's preferred to gradually roll out in batches so that we get the opportunity to address any unexpected outcome and correct course with limited costs. It is also a way to let developers digest the change without being too overwhelmed by it.

This is what I am experimenting in an enterprise account at $DAYJOB with evergreen: I rolled out in smaller orgs and incorporated the learning (and made some pull requests, for example #69), and as I am approaching bigger organizations I need a more ergonomic way to control the rollout batches.

The idea of this patch is to have evergreen rollout pull requests / issues at every schedule run to a fixed amount of repositories, and have it organically increase towards full coverage after every following run.

@zkoppert
Copy link
Member

Sounds great! Thanks for clarifying.

Let me know if you'd like support on resolving the ci failures.

@gbrindisi
Copy link
Contributor Author

@zkoppert hey Zack yes please I could use some help.
Locally I run make test and make lint and it passes alright, I am not very familiar with mypy

@zkoppert
Copy link
Member

I'm getting failing results locally for make test. 3 test failures.

FAILED test_env.py::TestEnv::test_get_env_vars_with_batch_size - AssertionError: Tuples differ: ('my_[251 chars]ndabot.yaml', None, False, [...
FAILED test_env.py::TestEnv::test_get_env_vars_with_no_batch_size - AssertionError: Tuples differ: ('my_[254 chars]bot.yaml', None, False, ['pr...
FAILED test_env.py::TestEnv::test_get_env_vars_with_repos_exempt_ecosystems - AssertionError: Tuples differ: ('my_[271 chars]False, ['private', 'public']...

I will take a look at the linter fixes first and come back to these.

Signed-off-by: Zack Koppert <zkoppert@github.com>
Signed-off-by: Zack Koppert <zkoppert@github.com>
Signed-off-by: Zack Koppert <zkoppert@github.com>
Copy link
Member

@zkoppert zkoppert left a comment

Choose a reason for hiding this comment

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

Super minor changes to pass the checks! 😅 I'll cut a release shortly after merging.

Thank @gbrindisi for this awesome addition of batching!

@zkoppert zkoppert merged commit 827ae2d into github:main Mar 25, 2024
4 checks passed
@gbrindisi
Copy link
Contributor Author

thanks @zkoppert !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants