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

Added default count for large commits #2006

Closed
wants to merge 1 commit into from

Conversation

ankitghosh
Copy link

We faced an issue where for some reason our commit history was huge around 45K, sentry-cli set-commits command was in a hung state for this use case.
When I dig deep I found initial-depth flag (default_count) was not being used to limit the commits get_commits_from_git function. Default limit set is 20.
So to fix that i have added a filter to take only default_count number of commits.

@ankitghosh
Copy link
Author

@loewenheim please can you take a look at this PR.

@szokeasaurusrex
Copy link
Member

Hi @ankitghosh, I have taken over from @loewenheim as the primary Sentry CLI maintainer.

While I appreciate the PR, it needs significant changes before we can consider merging it. default_count specifies the number of commits to include when the previous release's SHA cannot be found, and the --ignore-missing command line argument is provided. It is meant to only be used for this specific purpose. The default behavior is, and should continue to be, that we search for the previous commit with no limit to how far back we go. Implementing a new default limit would be a breaking change, since users might have releases with more than the default number of commits, and implementing a default limit would prevent them from being able to make such releases.

We can consider a different solution where we have a separate limit specifically for how far back we search for the commit. However, this limit would need to be disabled by default to avoid introducing a breaking change.

Since we need to rethink how we make this change entirely, I will close this PR for now. Please open a new issue1 referencing this PR, and we can continue our discussion there!

Footnotes

  1. For future reference, we typically triage issues more quickly than PRs, so I would always recommend opening an issue when you have a problem, so we can assist ASAP! You can always also open a PR and link the PR to the issue.

@ankitghosh
Copy link
Author

Created a new issue #2026

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