Skip to content

Conversation

@james-rl
Copy link
Contributor

Highlights:

  • Added ability to provide build context and named context mounts when building blueprints
  • There's a static method for invoking this and helpers for the object oriented SDKs
  • .dockerignore files are respected. Instead of importing the entire docker python SDK just for this feature there's some gnarly glob and string parsing to achieve the same effect.

Notes for Reviewers:

  • I went back and forth on the best way to expose the helpers for the SDKs. I felt like having a builder for blueprints was too different to other code, but I don't love the approach I ended up going with:
    build_context=build_ctx_obj.as_build_context(),

Comment on lines +1 to +3
"""
Helpers for `runloop_api_client`.
"""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

stop pytest from complaining when using resources from here

@adam-rl
Copy link
Contributor

adam-rl commented Nov 21, 2025

Do we want to merge the filtering that's done here (dockerignore) with the existing upload_from_dir method?

@james-rl
Copy link
Contributor Author

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.

@james-rl
Copy link
Contributor Author

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.

tar.add(path, arcname=".", recursive=True)
tar_buffer.seek(0)
return tar_buffer.read()
return build_docker_context_tar(path, ignore=ignore)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is the right abstraction, here, why are we talking about a docker context?

Copy link
Contributor

Choose a reason for hiding this comment

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

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

name: Optional[str] = None,
metadata: Optional[Dict[str, str]] = None,
ttl: Optional[timedelta] = None,
ignore: str | Path | Sequence[str] | None = None,
Copy link
Contributor

Choose a reason for hiding this comment

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

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]:
Copy link
Contributor

Choose a reason for hiding this comment

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

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"),
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made an interface for ignore types so you can have different .ignore files & patterns and added a DockerIgnoreMatcher type

Copy link
Contributor

@adam-rl adam-rl left a comment

Choose a reason for hiding this comment

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

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("*"):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we filtering files on the first level here? We can just call tf.add(root, arcname=".", filter=tar_filter) once right?

Copy link
Contributor

@sid-rl sid-rl left a comment

Choose a reason for hiding this comment

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

mostly comments about docstrings

@james-rl james-rl enabled auto-merge (squash) December 6, 2025 00:50
Copy link
Contributor

@sid-rl sid-rl left a comment

Choose a reason for hiding this comment

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

LGTM!

@james-rl james-rl merged commit 42849d6 into main Dec 6, 2025
7 checks passed
@james-rl james-rl deleted the james/ctxt-loader branch December 6, 2025 00:52
@stainless-app stainless-app bot mentioned this pull request Dec 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants