[TRUNK-16539] Use path in meta_json for internal bin #782
Conversation
Switches over to using the path in the meta_json to find the internal bin file. This does end up loading the whole tarball into memory in order to check for files.
|
😎 Merged successfully - details. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #782 +/- ##
==========================================
+ Coverage 72.88% 73.43% +0.54%
==========================================
Files 72 72
Lines 17369 17415 +46
==========================================
+ Hits 12660 12788 +128
+ Misses 4709 4627 -82 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bundle/src/bundler.rs
Outdated
| while let Some(entry) = entries.next().await { | ||
| let mut unwrapped_entry = entry?; | ||
| let path_str = unwrapped_entry | ||
| .path()? | ||
| .to_str() | ||
| .unwrap_or_default() | ||
| .to_owned(); | ||
| let mut bytes = Vec::new(); | ||
| unwrapped_entry.read_to_end(&mut bytes).await?; | ||
| file_entries.insert(path_str, bytes); | ||
| } |
There was a problem hiding this comment.
Why are we reading through every file? We know that we care about meta.json and we also know that the internal.bin will have internal in its name (whether that be the directory or the filename. Can we pre-filter based on that so we don't read everything? I'm concerned about some of our customers with large uploads.
There was a problem hiding this comment.
I was encountering issues passing around the entries, but this actually does cause issues for the context-py tests, so I'm going to need to switch away from it.
…load the two files we need
bundle/src/bundler.rs
Outdated
| if path_str == target_name { | ||
| let mut bytes = Vec::new(); | ||
| unwrapped_entry.read_to_end(&mut bytes).await?; | ||
| println!("bytes read {:?} from {:?}", bytes, path_str); |
There was a problem hiding this comment.
| println!("bytes read {:?} from {:?}", bytes, path_str); |
bundle/src/bundler.rs
Outdated
| ) -> anyhow::Result<(TestReport, VersionedBundle)> { | ||
| let extracted_files = | ||
| extract_files_from_tarball(input, &[META_FILENAME, INTERNAL_BIN_FILENAME]).await?; | ||
| println!("a"); |
There was a problem hiding this comment.
| println!("a"); |
| return value | ||
|
|
||
|
|
||
| def create_stream_from_meta( |
There was a problem hiding this comment.
Is this the same test but just moved down?
There was a problem hiding this comment.
Yeah, git's diff is misinterpreting what I did: the body of the baseMeta function was just a part of the body of test_parse_meta_from_tarball, but when I moved it up, git-diff decided it was a smaller diff to claim these functions had moved down, rather than to claim that the body of baseMeta had moved up.
4 participants
Switches over to using the path in the meta_json to find the internal bin file. This does end up loading the whole tarball into memory in order to check for files.