Skip to content

Add optional depth parameter to envd's ListDir #524

Merged
0div merged 13 commits intomainfrom
recursive-files-list-in-the-sdk-e2b-1902
Apr 16, 2025
Merged

Add optional depth parameter to envd's ListDir #524
0div merged 13 commits intomainfrom
recursive-files-list-in-the-sdk-e2b-1902

Conversation

@0div
Copy link
Copy Markdown
Contributor

@0div 0div commented Apr 1, 2025

Description

This envd change allows us to add an optional depth parameter to files.list method on the SDK.

The DX on the SDK side would look like this:

const files = await sandbox.files.list(dirName, { depth: 3 })
  • update pb spec
  • generate pb go files
  • update ListDir method

Test

  • infra integration test
  • SDK tests

@linear
Copy link
Copy Markdown

linear bot commented Apr 1, 2025

@0div 0div self-assigned this Apr 1, 2025
@0div 0div added the feature New feature label Apr 1, 2025
@0div 0div marked this pull request as ready for review April 1, 2025 02:44
mishushakov

This comment was marked as resolved.

@0div
Copy link
Copy Markdown
Contributor Author

0div commented Apr 2, 2025

I think this looks good to me, but keep in mind that "filepath.WalkDir reads an entire directory into memory before proceeding to walk that directory"

oh wow really ? where did you see this?

@mishushakov
Copy link
Copy Markdown
Member

oh wow really ? where did you see this?

https://pkg.go.dev/io/fs#WalkDir

@jakubno
Copy link
Copy Markdown
Member

jakubno commented Apr 2, 2025

I think this looks good to me, but keep in mind that "filepath.WalkDir reads an entire directory into memory before proceeding to walk that directory"

it reads stores names of the files / directories, which should be fine

@ValentaTomas
Copy link
Copy Markdown
Member

@mishushakov Can you merge if everything looks ok to you from both technical and DX perspective?
Then please finalize e2b-dev/E2B#649 and coordinate releasing of SDKs with the deployment of the envd change.

@ValentaTomas ValentaTomas removed their request for review April 8, 2025 20:17
Copy link
Copy Markdown
Member

@mishushakov mishushakov left a comment

Choose a reason for hiding this comment

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

Looks good, minor nits

Comment thread tests/integration/internal/tests/envd/filesystem_test.go
Comment thread tests/integration/internal/tests/envd/filesystem_test.go Outdated
Comment thread tests/integration/README.md Outdated
Comment thread tests/integration/README.md
Comment thread packages/envd/spec/filesystem/filesystem.proto
@0div 0div requested a review from mishushakov April 14, 2025 22:42
Copy link
Copy Markdown
Member

@mishushakov mishushakov left a comment

Choose a reason for hiding this comment

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

Make tests explicit and the rest looks good

Comment thread tests/integration/internal/tests/envd/filesystem_test.go
@0div 0div merged commit 414c0c9 into main Apr 16, 2025
9 of 10 checks passed
@0div 0div deleted the recursive-files-list-in-the-sdk-e2b-1902 branch April 16, 2025 19:34
0div added a commit to e2b-dev/E2B that referenced this pull request May 5, 2025
# Description

Do not merge before e2b-dev/infra#524 is merged
and deployed.

The DX would look like this:
```js
const files = await sandbox.files.list(dirName, { depth: 3 })
```
- [x] update `envd` pb spec 
- [x] generate for `js-sdk`
- [x] update `js-sdk` `file.list` method to include optional depth param
- [x] update `js-sdk` tests
- [x] generate for `python-sdk`
- [x] update `python-sdk` `file.list` method to include optional depth
param
  - [x] sync
  - [x] async
- [x] update `python-sdk` tests
  - [x] sync
  - [x] async

---------

Co-authored-by: Mish <10400064+mishushakov@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants