feat(SDK): Build context helpers for the python clients and SDKs#684
feat(SDK): Build context helpers for the python clients and SDKs#684
Conversation
…en creating a blueprint
| """ | ||
| Helpers for `runloop_api_client`. | ||
| """ |
There was a problem hiding this comment.
stop pytest from complaining when using resources from here
|
Do we want to merge the filtering that's done here (dockerignore) with the existing |
|
After today's meeting (11/21/2025) we decided that this approach is going to be hard to support and we probably -- probably -- want to enable a slightly different path that allows users to push images they build to our systems. I'm keeping this PR open for now until we get customer feedback. |
|
it's back! After looking at how daytona and modal support harbor for tb2 it seems like this is now table stakes for sandbox providers. |
src/runloop_api_client/sdk/async_.py
Outdated
| tar.add(path, arcname=".", recursive=True) | ||
| tar_buffer.seek(0) | ||
| return tar_buffer.read() | ||
| return build_docker_context_tar(path, ignore=ignore) |
There was a problem hiding this comment.
I don't think this is the right abstraction, here, why are we talking about a docker context?
There was a problem hiding this comment.
Also I was hoping we could just add the filter parameter to tar.add: https://docs.python.org/3/library/tarfile.html#tarfile.TarFile.add
src/runloop_api_client/sdk/async_.py
Outdated
| name: Optional[str] = None, | ||
| metadata: Optional[Dict[str, str]] = None, | ||
| ttl: Optional[timedelta] = None, | ||
| ignore: str | Path | Sequence[str] | None = None, |
There was a problem hiding this comment.
You didn't want to introduce a new type that handles DockerIgnore?
That way the logic of the docker ignore file can live in a separate class that emits patterns compatible with the tarfile ignore property?
| return pattern | ||
|
|
||
|
|
||
| def read_ignorefile(path: Optional[Path]) -> list[str]: |
There was a problem hiding this comment.
I thought we discussed making an object for this so that it encapsulates the pattern parsing.
Ideally we have a class like DockerIgnoreMatcher("path/to/dockerignore") and it implements some interface that allows us to assign it to the upload_from_dir method:
await runloop.objects.upload_from_dir(
"path/to/dir",
name = "my-context",
ignore=DockerIgnoreMatcher("path/to/dir/.dockerignore"),
)There was a problem hiding this comment.
I made an interface for ignore types so you can have different .ignore files & patterns and added a DockerIgnoreMatcher type
adam-rl
left a comment
adam-rl
left a comment
There was a problem hiding this comment.
It's hard to see what's used, would you mind deleting the code that is not referenced anymore?
I think the build strategy stuff, and docker context stuff is unused.
| root = root.resolve() | ||
| buf = io.BytesIO() | ||
| with tarfile.open(mode="w:gz", fileobj=buf) as tf: | ||
| for file_path in root.rglob("*"): |
There was a problem hiding this comment.
Why are we filtering files on the first level here? We can just call tf.add(root, arcname=".", filter=tar_filter) once right?
…t a bit more ergonomic
sid-rl
left a comment
sid-rl
left a comment
There was a problem hiding this comment.
mostly comments about docstrings
3 participants
Highlights:
.dockerignorefiles are respected. Instead of importing the entiredockerpython SDK just for this feature there's some gnarly glob and string parsing to achieve the same effect.Notes for Reviewers:
build_context=build_ctx_obj.as_build_context(),