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

Image copy support #1122

Merged
merged 22 commits into from
Dec 20, 2024
Merged

Image copy support #1122

merged 22 commits into from
Dec 20, 2024

Conversation

j3m7
Copy link
Contributor

@j3m7 j3m7 commented Dec 18, 2024

Context

This adds COPY support for images as well as setting up a foundation for supporting more build operations.

What

Refactors image building to execute on a list of BuildOps and ports all existing image building functionality. Also adds support for cloud bases image service.

Testing

Run indexify-cli build-default-image and indexify-cli build-image with existing image definitions. Viable images should be built.

Contribution Checklist

  • If the python-sdk was changed, please run make fmt in python-sdk/.
  • If the server was changed, please run make fmt in server/.
  • Make sure all PR Checks are passing.

Julio Martinez and others added 6 commits December 17, 2024 09:00
- Adds a simple extensible framework for supporting multiple build operations. Right now only RUN and COPY are implemented.
- Refactored Dockerfile generation into the Image class. Rewired existing builders to use this.
- Re-implemented `build-platform-image` to use new builder service API.
@j3m7 j3m7 marked this pull request as ready for review December 19, 2024 22:47
@@ -161,6 +158,32 @@ def build_image(
_create_image(obj, python_sdk_path)


@app.command(help="Build platform images for function names")
Copy link
Collaborator

Choose a reason for hiding this comment

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

@j3m7 this needs to be in the Tensorlake cli, and wrapped in Tensorlake deploy

for OSS, we could translate .copy to COPY in the local build context

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, it's final home will be in the tensorlake cli. added it here so we could start leveraging this functionality on our internal build pipelines before writing the tensorlake cli.

The .copy() calls are already rendered as COPY for both local and remote build contexts.

Copy link
Member

Choose a reason for hiding this comment

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

We discussed the approach of postponing creating the tensorlake CLI + SDK to later since it is not needed for document AI.

Since everything will be open source, having it live here for now is seen as a good way for us to make progress and not have to setup a new repo, new pipy, new CI.

image_hasher.update(base_image.clone());
image_hasher.update(run_strs.clone().join(""));
image_hasher.update(sdk_version.clone().unwrap_or("".to_string())); // Igh.....
let mut compat_image_hash: String = "".to_string();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@seriousben as discussed we preserve backwards compat while accepting blank image_hashes.

Co-authored-by: Benjamin Boudreau <[email protected]>
res = client.get(
f"{service_endpoint}/builds/{build.id}", headers=headers
)
build = Build.model_validate(res.json())
Copy link
Member

Choose a reason for hiding this comment

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

Is this where we'll be able to get the logs and display them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After the build is completed another call will need to be made to retrieve the logs, that is not implemented at this time.


import docker.api.build
case "failed":
print(f"Building failed, please see logs for details")
Copy link
Member

Choose a reason for hiding this comment

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

What is the current expectation when this fails? Is there a way to get the logs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not for the client side, they are logged by the builder task. A subsequent feature would be to store them in the blob store and add some API tissue to retrieve them.

Copy link
Member

@seriousben seriousben left a comment

Choose a reason for hiding this comment

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

LGTM, would be useful to know what you are planning to do for logs in the perspective that a document ai image changes introduce an issue.

@j3m7 j3m7 merged commit ab93241 into main Dec 20, 2024
8 checks passed
@j3m7 j3m7 deleted the image-copy-support branch December 20, 2024 21:53
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.

3 participants