fix: Improve sandbox.git async/sync parity#1110
Conversation
Directly related to #1108. Please check the issue description for more details. tldr: `.restore()` and .`reset()` functions are missing in the `AsyncSandbox` instance also added test to check parity of methods/signatures between sync and async git instances other potential concerns: - tests are only ran for the sync variant across the python-sdk - if there are any mismatches in the async logic (not only git i believe), the tests won't catch it
🦋 Changeset detectedLatest commit: b86e5ae The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
djeebus
left a comment
There was a problem hiding this comment.
Only comment is that we probably want to move things that are not intended to be part of the public API into modules that start with _. Specifically I'm thinking of all the functions in e2b.sandbox.git.args; if we want people to call them via sandbox_sync.Git methods, we should probably rename e2b.sandbox.git to e2b.sandbox._git, indicating that functions in there might get refactored, renamed, or removed without warning.
There was a problem hiding this comment.
can we do this test all of our classes? like Sandbox, SandboxAsync, Template, TemplateAsync
|
Do we need to do same changes as in this PR in our JS SDKs as well? |
|
hmm, we can add the sync with the JS but maybe a bit more of a pain; the user had provided the improvement to the python side of things so i think leaving it alone as a python only improvement fine to reduce scope creep |
|
@matthewlouisbrockman i mean the changes to the structure like what is in which file? |
Fixes issue with missing
restoreandresetfunctionality on the async git sandboxes.Consolidate shared git helpers by moving remote URL argument construction and
parsing into the git utilities package. Sync and async git modules now reuse
the same builders where possible, with tests to ensure no drift.