Skip to content

Commit ccb71af

Browse files
perf(sourcemaps): Elminate unnecessary HTTP call (#2913)
### Description In some cases, we were making an HTTP call to list release files to determine whether certain sources were already uploaded. However, if the sources are uploaded as artifact bundles (the default), they would not get returned from this endpoint, so the HTTP call is unnecessary. Removing the call should speed up the `sourcemaps upload` command somewhat. ### Issues - Resolves #2912 - Resolves CLI-213
1 parent 3f51037 commit ccb71af

File tree

13 files changed

+19
-103
lines changed

13 files changed

+19
-103
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,16 @@
1010

1111
- Deprecated the `upload-proguard` subcommand's `--platform` flag ([#2863](https://github.com/getsentry/sentry-cli/pull/2863)). This flag appears to have been a no-op for some time, so we will remove it in the next major.
1212
- Deprecated the `upload-proguard` subcommand's `--android-manifest` flag ([#2891](https://github.com/getsentry/sentry-cli/pull/2891)). This flag appears to have been a no-op for some time, so we will remove it in the next major.
13+
- Deprecated the `sentry-cli sourcemaps upload` command's `--no-dedupe` flag ([#2913](https://github.com/getsentry/sentry-cli/pull/2913)). The flag was no longer relevant for sourcemap uploads to modern Sentry servers; the flag is now a no-op.
1314

1415
### Fixes
1516

1617
- Fix autofilled git base metadata (`--base-ref`, `--base-sha`) when using the `build upload` subcommand in git repos. Previously this worked only in the contexts of GitHub workflows ([#2897](https://github.com/getsentry/sentry-cli/pull/2897), [#2898](https://github.com/getsentry/sentry-cli/pull/2898)).
1718

19+
### Performance
20+
21+
- Slightly sped up the `sentry-cli sourcemaps upload` command by elminating an HTTP request to the Sentry server, which was not required in most cases ([#2913](https://github.com/getsentry/sentry-cli/pull/2913)).
22+
1823
## 2.57.0
1924

2025
### New Features

src/commands/debug_files/bundle_jvm.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
6565
note: None,
6666
wait: true,
6767
max_wait: DEFAULT_MAX_WAIT,
68-
dedupe: false,
6968
chunk_upload_options: chunk_upload_options.as_ref(),
7069
};
7170
let path = matches.get_one::<PathBuf>("path").unwrap();

src/commands/files/upload.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,6 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
159159
note: None,
160160
wait,
161161
max_wait,
162-
dedupe: false,
163162
chunk_upload_options: chunk_upload_options.as_ref(),
164163
};
165164

src/commands/react_native/appcenter.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,6 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
202202
note: None,
203203
wait,
204204
max_wait,
205-
dedupe: false,
206205
chunk_upload_options: chunk_upload_options.as_ref(),
207206
})?;
208207
}
@@ -221,7 +220,6 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
221220
note: None,
222221
wait,
223222
max_wait,
224-
dedupe: false,
225223
chunk_upload_options: chunk_upload_options.as_ref(),
226224
})?;
227225
}

src/commands/react_native/gradle.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,6 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
129129
note: None,
130130
wait,
131131
max_wait,
132-
dedupe: false,
133132
chunk_upload_options: chunk_upload_options.as_ref(),
134133
})?;
135134
}
@@ -143,7 +142,6 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
143142
note: None,
144143
wait,
145144
max_wait,
146-
dedupe: false,
147145
chunk_upload_options: chunk_upload_options.as_ref(),
148146
})?;
149147
}

src/commands/react_native/xcode.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,6 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
351351
note: None,
352352
wait,
353353
max_wait,
354-
dedupe: false,
355354
chunk_upload_options: chunk_upload_options.as_ref(),
356355
})?;
357356
} else {
@@ -387,7 +386,6 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
387386
note: None,
388387
wait,
389388
max_wait,
390-
dedupe: false,
391389
chunk_upload_options: chunk_upload_options.as_ref(),
392390
})?;
393391
}
@@ -401,7 +399,6 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
401399
note: None,
402400
wait,
403401
max_wait,
404-
dedupe: false,
405402
chunk_upload_options: chunk_upload_options.as_ref(),
406403
})?;
407404
}

src/commands/sourcemaps/upload.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -193,11 +193,8 @@ pub fn make_command(command: Command) -> Command {
193193
Arg::new("no_dedupe")
194194
.long("no-dedupe")
195195
.action(ArgAction::SetTrue)
196-
.help(
197-
"Skip artifacts deduplication prior to uploading. \
198-
This will force all artifacts to be uploaded, \
199-
no matter whether they are already present on the server.",
200-
),
196+
.hide(true)
197+
.help("[DEPRECATED] This flag has no effect and is scheduled for removal."),
201198
)
202199
.arg(
203200
Arg::new("extensions")
@@ -419,6 +416,13 @@ fn process_sources_from_paths(
419416
}
420417

421418
pub fn execute(matches: &ArgMatches) -> Result<()> {
419+
if matches.get_flag("no_dedupe") {
420+
log::warn!(
421+
"[DEPRECATION NOTICE] The --no-dedupe flag is deprecated and has no \
422+
effect. It will be removed in the next major version."
423+
);
424+
}
425+
422426
let config = Config::current();
423427
let version = config.get_release_with_legacy_fallback(matches).ok();
424428
let org = config.get_org(matches)?;
@@ -457,7 +461,6 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
457461
note: matches.get_one::<String>("note").map(String::as_str),
458462
wait,
459463
max_wait,
460-
dedupe: !matches.get_flag("no_dedupe"),
461464
chunk_upload_options: chunk_upload_options.as_ref(),
462465
};
463466

src/utils/file_upload.rs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use crate::api::NewRelease;
2828
use crate::api::{Api, ChunkServerOptions, ChunkUploadCapability};
2929
use crate::constants::DEFAULT_MAX_WAIT;
3030
use crate::utils::chunks::{upload_chunks, Chunk, ASSEMBLE_POLL_INTERVAL};
31-
use crate::utils::fs::{get_sha1_checksum, get_sha1_checksums, TempFile};
31+
use crate::utils::fs::{get_sha1_checksums, TempFile};
3232
use crate::utils::non_empty::NonEmptySlice;
3333
use crate::utils::progress::{ProgressBar, ProgressBarMode, ProgressStyle};
3434

@@ -95,7 +95,6 @@ pub struct UploadContext<'a> {
9595
pub note: Option<&'a str>,
9696
pub wait: bool,
9797
pub max_wait: Duration,
98-
pub dedupe: bool,
9998
pub chunk_upload_options: Option<&'a ChunkServerOptions>,
10099
}
101100

@@ -308,11 +307,6 @@ impl SourceFile {
308307
source_file
309308
}
310309

311-
/// Calculates and returns the SHA1 checksum of the file.
312-
pub fn checksum(&self) -> Result<Digest> {
313-
get_sha1_checksum(&**self.contents)
314-
}
315-
316310
/// Returns the value of the "debug-id" header.
317311
pub fn debug_id(&self) -> Option<&String> {
318312
self.headers.get("debug-id")
@@ -902,7 +896,6 @@ mod tests {
902896
note: None,
903897
wait: false,
904898
max_wait: DEFAULT_MAX_WAIT,
905-
dedupe: true,
906899
chunk_upload_options: None,
907900
};
908901

src/utils/sourcemaps.rs

Lines changed: 1 addition & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,18 @@ use std::io::Write as _;
66
use std::mem;
77
use std::path::{Path, PathBuf};
88
use std::str;
9-
use std::str::FromStr as _;
109
use std::sync::Arc;
1110

1211
use anyhow::{anyhow, bail, Context as _, Error, Result};
1312
use console::style;
1413
use indicatif::ProgressStyle;
1514
use log::{debug, info, warn};
1615
use sentry::types::DebugId;
17-
use sha1_smol::Digest;
1816
use sourcemap::{DecodedMap, SourceMap};
1917
use symbolic::debuginfo::js::discover_sourcemaps_location;
2018
use symbolic::debuginfo::sourcebundle::SourceFileType;
2119
use url::Url;
2220

23-
use crate::api::Api;
2421
use crate::utils::file_search::ReleaseFileMatch;
2522
use crate::utils::file_upload::{
2623
initialize_legacy_release_upload, FileUpload, SourceFile, SourceFiles, UploadContext,
@@ -666,54 +663,6 @@ impl SourceMapProcessor {
666663
}
667664
}
668665

669-
/// Flags the collected sources whether they have already been uploaded before
670-
/// (based on their checksum), and returns the number of files that *do* need an upload.
671-
fn flag_uploaded_sources(&mut self, context: &UploadContext<'_>) -> usize {
672-
let mut files_needing_upload = self.sources.len();
673-
674-
if !context.dedupe {
675-
return files_needing_upload;
676-
}
677-
678-
// This endpoint only supports at most one project, and a release is required.
679-
// If the upload contains multiple projects or no release, we do not use deduplication.
680-
let (project, release) = match (context.projects.as_deref(), context.release) {
681-
(Some([project]), Some(release)) => (Some(project.as_str()), release),
682-
(None, Some(release)) => (None, release),
683-
_ => return files_needing_upload,
684-
};
685-
686-
let mut sources_checksums: Vec<_> = self
687-
.sources
688-
.values()
689-
.filter_map(|s| s.checksum().map(|c| c.to_string()).ok())
690-
.collect();
691-
692-
// Checksums need to be sorted in order to satisfy integration tests constraints.
693-
sources_checksums.sort();
694-
695-
let api = Api::current();
696-
697-
if let Ok(artifacts) = api.authenticated().and_then(|api| {
698-
api.list_release_files_by_checksum(context.org, project, release, &sources_checksums)
699-
}) {
700-
let already_uploaded_checksums: HashSet<_> = artifacts
701-
.into_iter()
702-
.filter_map(|artifact| Digest::from_str(&artifact.sha1).ok())
703-
.collect();
704-
705-
for source in self.sources.values_mut() {
706-
if let Ok(checksum) = source.checksum() {
707-
if already_uploaded_checksums.contains(&checksum) {
708-
source.already_uploaded = true;
709-
files_needing_upload -= 1;
710-
}
711-
}
712-
}
713-
}
714-
files_needing_upload
715-
}
716-
717666
/// Uploads all files, and on success, returns the number of files that were
718667
/// uploaded, wrapped in Ok()
719668
pub fn upload(&mut self, context: &UploadContext<'_>) -> Result<usize> {
@@ -752,7 +701,7 @@ impl SourceMapProcessor {
752701
}
753702
}
754703
}
755-
let files_needing_upload = self.flag_uploaded_sources(context);
704+
let files_needing_upload = self.sources.len();
756705
if files_needing_upload > 0 {
757706
let mut uploader = FileUpload::new(context);
758707
uploader.files(&self.sources);

tests/integration/_cases/sourcemaps/sourcemaps-upload-help.trycmd

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,6 @@ Options:
7474
Path to the application bundle (indexed, file, or regular)
7575
--bundle-sourcemap <BUNDLE_SOURCEMAP>
7676
Path to the bundle sourcemap
77-
--no-dedupe
78-
Skip artifacts deduplication prior to uploading. This will force all artifacts to be
79-
uploaded, no matter whether they are already present on the server.
8077
-x, --ext <EXT>
8178
Set the file extensions that are considered for upload. This overrides the default
8279
extensions. To add an extension, all default extensions must be repeated. Specify once per

0 commit comments

Comments
 (0)