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

fix: Break out of the loop when we reach cursor limit for list_release_files #1337

Merged
merged 1 commit into from Sep 23, 2022

Conversation

kamilogorek
Copy link
Contributor

@kamilogorek kamilogorek commented Sep 22, 2022

Workaround for the small regression introduced in #1275 that happens in usecases where over 20k of files are uploaded for a single release.

Potential improvement is explained in the comment:

Instead of breaking out of the loop here when we reach the limit
(200 pages of 100 items, for a total of 20_000 files), we should send a list of hashes
to the server, perform an intersection there, and change the upload mechanism
to leave out only those files that were gave back to us by the server.
This would also limit number of API requests for deduping from N=[1:200] to 1.

Related DIFs functionality that we can reuse here:

  1. https://github.com/getsentry/sentry/blob/f9bf16e7d663c780c1e0c0378ae92f2a93aa326b/src/sentry/api/urls.py#L1941-L1945
  2. https://github.com/getsentry/sentry/blob/f9bf16e7d663c780c1e0c0378ae92f2a93aa326b/src/sentry/api/endpoints/debug_files.py#L248-L255
  3. sentry-cli/src/api.rs

    Lines 1005 to 1030 in 2cac02b

    /// Given a list of checksums for DIFs, this returns a list of those
    /// that do not exist for the project yet.
    pub fn find_missing_dif_checksums<I>(
    &self,
    org: &str,
    project: &str,
    checksums: I,
    ) -> ApiResult<HashSet<Digest>>
    where
    I: IntoIterator<Item = Digest>,
    {
    let mut url = format!(
    "/projects/{}/{}/files/dsyms/unknown/?",
    PathArg(org),
    PathArg(project)
    );
    for (idx, checksum) in checksums.into_iter().enumerate() {
    if idx > 0 {
    url.push('&');
    }
    url.push_str("checksums=");
    url.push_str(&checksum.to_string());
    }
    let state: MissingChecksumsResponse = self.get(&url)?.convert()?;
    Ok(state.missing)

Fixes #1306

@kamilogorek kamilogorek requested a review from a team September 22, 2022 13:17
}
}
None
self.configurations
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Make clippy happy"

@kamilogorek kamilogorek merged commit ebae541 into master Sep 23, 2022
@kamilogorek kamilogorek deleted the break-files-loop branch September 23, 2022 08:50
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.

Pagination offset too large (http status: 400)
2 participants