Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add recursive directory watch #488

Merged
merged 17 commits into from
Jan 28, 2025
Merged

Conversation

dobrac
Copy link
Contributor

@dobrac dobrac commented Dec 2, 2024

Adds recursive directory watch to the SDK and documentation section. Requires merging and deploying e2b-dev/infra#210 first.

Proposed API change:

JS

await sandbox.files.watchDir(dirname, handler, {
  recursive: true
})

Python

sandbox.files.watch_dir(dirname, recursive=True)

Notes

Python SDK was generated using buf.build/protocolbuffers/python:v28.3. The runtime check was removed for now. Further tests might be required to make sure the runtime is at least the generated version before including the runtime check.
Changing the following in the file spec/envd/buf-python.gen.yaml (generator) allows using different protoc-gen-python version:
From: plugin: python
To: plugin: buf.build/protocolbuffers/python:v28.3

Copy link

changeset-bot bot commented Dec 2, 2024

🦋 Changeset detected

Latest commit: 13f0c78

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@e2b/python-sdk Patch
e2b Patch

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

@mlejva mlejva requested a review from 0div December 2, 2024 21:34
@dobrac dobrac marked this pull request as ready for review December 2, 2024 23:10
0div
0div previously requested changes Dec 3, 2024
Copy link
Contributor

@0div 0div left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good from a code perspective 👍

From a product perspective, do you think it would be better not to make recursive an option and just always watch recursively ? Also taking into account that the instructions said:

Make as few changes to the communication between envd and SDKs as possible

@ValentaTomas ValentaTomas added feature New feature or request sdk Improvements or additions to SDKs labels Dec 10, 2024
@dobrac dobrac force-pushed the watch-dir-recursive branch from 666b3d4 to 724e857 Compare December 19, 2024 08:56
Copy link
Member

@jakubno jakubno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The envd version check is missing in Python SDK

Copy link
Member

@jakubno jakubno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some last polishing, I will merge after we merge the envd part

packages/js-sdk/src/sandbox/index.ts Outdated Show resolved Hide resolved
packages/js-sdk/tests/sandbox/files/watch.test.ts Outdated Show resolved Hide resolved
packages/js-sdk/tests/sandbox/files/watch.test.ts Outdated Show resolved Hide resolved
Copy link
Member

I think the envd part is already merged, right?

@0div
Copy link
Contributor

0div commented Jan 24, 2025

I think the envd part is already merged, right?

looks like it e2b-dev/infra#233

@jakubno jakubno force-pushed the watch-dir-recursive branch from f4369e1 to 8c52e73 Compare January 28, 2025 19:28
@jakubno jakubno merged commit 4fb4470 into e2b-dev:main Jan 28, 2025
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request sdk Improvements or additions to SDKs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants