Upload a chunk at a time

[chromy]

Update sentry_client.py to not need the whole file in memory at once.

sentry_client.py was reading whole files at once to chunk them up and send them
to the monolith. This was bad for build distribution which sends a whole app back
to the monolith since it caused large spikes in memory usage as the apps can be ~X00mb.

While here we make a number of simplifications:

• Replacing the hand rolled multipart/form-data encoding (which was added to handle the rpc0 auth)
  with the built in requests encoding + a custom auth provider.
• Add responses - Sentry's own library for testing
  requests which seems useful here and allows less fragile mocking in the tests.
• Adding support for taking the secret at construction time rather than only via an ENV variable
  which again allows for simpler tests.
• Update tests to use example.com rather than test.sentry.io - to avoid any issues if the requests
  go though for real.

[codecov]

Codecov Report

❌ Patch coverage is 87.75510% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.23%. Comparing base (c8c5a85) to head (577433b).
⚠️ Report is 17 commits behind head on main.

Files with missing lines	Patch %	Lines
src/launchpad/sentry_client.py	81.53%	9 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #296      +/-   ##
==========================================
+ Coverage   77.77%   78.23%   +0.46%     
==========================================
  Files         142      142              
  Lines       11143    11152       +9     
  Branches     1185     1184       -1     
==========================================
+ Hits         8666     8725      +59     
+ Misses       2057     1999      -58     
- Partials      420      428       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[graphite-app]

The assembly process no longer attempts to re-upload missing chunks when they're reported. This removes an important recovery mechanism from the original implementation.

In the previous code, when chunks were missing, it would:

logger.info(f"Re-uploading {len(missing)} missing chunks")
if not self._upload_chunks(org, chunks, missing):
    logger.warning(f"Some chunks failed to re-upload on attempt {attempt + 1}")

The new implementation simply logs a warning and returns the error result immediately. This could lead to failed uploads that might have succeeded with the retry mechanism. Consider restoring the chunk re-upload functionality to maintain the same resilience as the original implementation.

Suggested change

	if missing:
	logger.warning(f"{len(missing)} chunks failed to upload")
	else:
	logger.warning("Assembly failed but no missing chunks reported")
	return result
	logger.info(f"Re-uploading {len(missing)} missing chunks")
	if not self._upload_chunks(org, chunks, missing):
	logger.warning(f"Some chunks failed to re-upload on attempt {attempt + 1}")
	return result
	if missing:
	logger.info(f"Re-uploading {len(missing)} missing chunks")
	if not self._upload_chunks(org, chunks, missing):
	logger.warning(f"{len(missing)} chunks failed to re-upload")
	else:
	logger.warning("Assembly failed but no missing chunks reported")
	return result

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.

[NicoHinderling]

+1

[chromy]

I want to put this in loop so it starts with an assemble then only uploads the missing chunks. I'll do it in a follow up if that works for you.

[NicoHinderling]

is this intentional? i dont see any changes to service.py

[chromy]

Right now only the tests use this but ideally I think it would be better to pass around file objects rather than paths. That way it's much harder to accidentally either fail to delete something (so leaking the disk space till the process dies) or deleting something too soon (and causing crashes).

I want to update the calling code to pass the arguments as files, then we can delete the 'file_path' handling (here + all of _upload_path_with_assembly) but not in this PR.

[NicoHinderling]

+1