From 40ff371e698d6e0cf023dbf87a9668d8f5a31022 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Thu, 30 Mar 2023 14:45:00 +0200 Subject: [PATCH 1/2] Skip empty SourceMap uploads This avoids uploading empty (but still unique until we pull in https://github.com/getsentry/symbolic/pull/778) artifact / release bundles if all the source maps included in them have already been uploaded before. --- src/utils/file_upload.rs | 6 +++--- src/utils/sourcemaps.rs | 28 +++++++++++++++++++--------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/utils/file_upload.rs b/src/utils/file_upload.rs index 7a53adac0c..fbcaca92c9 100644 --- a/src/utils/file_upload.rs +++ b/src/utils/file_upload.rs @@ -300,7 +300,7 @@ fn upload_files_chunked( )); upload_chunks(&chunks, options, progress_style)?; - println!("{} Uploaded release files to Sentry", style(">").dim(),); + println!("{} Uploaded release files to Sentry", style(">").dim()); let progress_style = ProgressStyle::default_spinner().template("{spinner} Processing files..."); @@ -443,6 +443,8 @@ fn build_artifact_bundle(context: &UploadContext, files: &SourceFiles) -> Result bundle.finish()?; + pb.finish_with_duration("Bundling"); + println!( "{} Bundled {} {} for upload", style(">").dim(), @@ -453,8 +455,6 @@ fn build_artifact_bundle(context: &UploadContext, files: &SourceFiles) -> Result } ); - pb.finish_with_duration("Bundling"); - Ok(archive) } diff --git a/src/utils/sourcemaps.rs b/src/utils/sourcemaps.rs index 18152be543..aa389b60ce 100644 --- a/src/utils/sourcemaps.rs +++ b/src/utils/sourcemaps.rs @@ -591,14 +591,18 @@ impl SourceMapProcessor { Ok(()) } - fn flag_uploaded_sources(&mut self, context: &UploadContext<'_>) { + /// Flags the collected sources whether they have already been uploaded before + /// (based on their checksum), and returns the number of files that *do* need an upload. + fn flag_uploaded_sources(&mut self, context: &UploadContext<'_>) -> usize { + let mut files_needing_upload = self.sources.len(); + // TODO: this endpoint does not exist for non release based uploads if !context.dedupe { - return; + return files_needing_upload; } let release = match context.release { Some(release) => release, - None => return, + None => return files_needing_upload, }; let mut sources_checksums: Vec<_> = self @@ -618,8 +622,8 @@ impl SourceMapProcessor { release, &sources_checksums, ) { - let already_uploaded_checksums: Vec<_> = artifacts - .iter() + let already_uploaded_checksums: HashSet<_> = artifacts + .into_iter() .filter_map(|artifact| Digest::from_str(&artifact.sha1).ok()) .collect(); @@ -627,20 +631,26 @@ impl SourceMapProcessor { if let Ok(checksum) = source.checksum() { if already_uploaded_checksums.contains(&checksum) { source.already_uploaded = true; + files_needing_upload -= 1; } } } } + files_needing_upload } /// Uploads all files pub fn upload(&mut self, context: &UploadContext<'_>) -> Result<()> { initialize_legacy_release_upload(context)?; self.flush_pending_sources(); - self.flag_uploaded_sources(context); - let mut uploader = FileUpload::new(context); - uploader.files(&self.sources); - uploader.upload()?; + let files_needing_upload = self.flag_uploaded_sources(context); + if files_needing_upload > 0 { + let mut uploader = FileUpload::new(context); + uploader.files(&self.sources); + uploader.upload()?; + } else { + println!("{} Nothing to upload", style(">").dim()); + } self.dump_log("Source Map Upload Report"); Ok(()) } From 37661babac80c6154a816b425ced19efa49c2a33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Thu, 30 Mar 2023 16:53:12 +0200 Subject: [PATCH 2/2] Dont print upload report for empty uploads and update snapshots --- src/utils/sourcemaps.rs | 2 +- .../releases/releases-files-upload-sourcemaps.trycmd | 11 +---------- tests/integration/sourcemaps/upload.rs | 5 ++--- 3 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/utils/sourcemaps.rs b/src/utils/sourcemaps.rs index aa389b60ce..58774b6546 100644 --- a/src/utils/sourcemaps.rs +++ b/src/utils/sourcemaps.rs @@ -648,10 +648,10 @@ impl SourceMapProcessor { let mut uploader = FileUpload::new(context); uploader.files(&self.sources); uploader.upload()?; + self.dump_log("Source Map Upload Report"); } else { println!("{} Nothing to upload", style(">").dim()); } - self.dump_log("Source Map Upload Report"); Ok(()) } diff --git a/tests/integration/_cases/releases/releases-files-upload-sourcemaps.trycmd b/tests/integration/_cases/releases/releases-files-upload-sourcemaps.trycmd index 399f64591f..f66da7a86f 100644 --- a/tests/integration/_cases/releases/releases-files-upload-sourcemaps.trycmd +++ b/tests/integration/_cases/releases/releases-files-upload-sourcemaps.trycmd @@ -3,15 +3,6 @@ $ sentry-cli releases files wat-release upload-sourcemaps --url-prefix '~/assets ? success > Rewriting sources > Adding source map references -> Bundled 0 files for upload -> Uploaded release files to Sentry -> File upload complete (processing pending on server) -> Organization: wat-org -> Project: wat-project -> Release: wat-release -> Dist: None -> Upload type: release bundle - -Source Map Upload Report +> Nothing to upload ``` diff --git a/tests/integration/sourcemaps/upload.rs b/tests/integration/sourcemaps/upload.rs index 332ac9ac6f..f5f006b437 100644 --- a/tests/integration/sourcemaps/upload.rs +++ b/tests/integration/sourcemaps/upload.rs @@ -146,8 +146,8 @@ fn command_sourcemaps_upload_modern() { } #[test] -fn command_releases_files_upload_sourcemap() { - let upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Legacy); +fn command_sourcemaps_upload_empty() { + let _upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Legacy); let _files = mock_endpoint( EndpointOptions::new( "GET", @@ -157,5 +157,4 @@ fn command_releases_files_upload_sourcemap() { .with_response_body("[]"), ); register_test("releases/releases-files-upload-sourcemaps.trycmd"); - assert_endpoints(&upload_endpoints); }