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.

Sign up for GitHub

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

Jump to bottom

Fix build to include layers that are single files #55

Merged
merged 2 commits into from
Mar 1, 2024
Merged

Fix build to include layers that are single files #55

merged 2 commits into from
Mar 1, 2024

Conversation

amisevsk
Copy link
Contributor

@amisevsk amisevsk commented Mar 1, 2024

Description

  • Fixup an issue where a Kitfile without a model would deserialize to having empty models (meaning name, path, etc. were empty strings). Instead, store model as pointer to allow it to be nil if not present
  • Rework build process to support both layers defined as single files and entire directories. We should be able to use both path: models/ and path: models/my-model.onnx now.
  • Write to a temporary file while building instead of loading entire gzip into an in-memory buffer

@amisevsk
Fixup issue during build if Kitfile does not define models
bc96292 
Since switching from a list of models to a single model per Kitfile,
Kitfiles that do not define models will deserialize to a Kitfile with an
empty model.

To avoid this, we change the `model` field on Kitfile to a pointer so
that it deserializes to nil when not present.
@amisevsk
Rework build process to support both directories and single files
ba47d3a 
Rework kit build to handle both layers specified as directories and as
regular files. In particular, we need to be able to package and extract
both

  model:
    path: ./model (directory)

and

  model:
    path: ./model.onnx (file)

This requires significant reworking of the build process as previously
we assumed we could always 1) create a directory and 2) extract files
into it.

In addition, this commit changes the build process to stream the build
to a temporary file rather than an in-memory buffer to avoid requiring a
lot of memory.
@amisevsk amisevsk requested a review from gorkem March 1, 2024 15:00
gorkem
gorkem approved these changes Mar 1, 2024
View reviewed changes
Copy link
Contributor

@gorkem gorkem left a comment

Choose a reason for hiding this comment

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

LGTM

@amisevsk amisevsk merged commit d7529ae into main Mar 1, 2024
1 check passed
@amisevsk amisevsk deleted the fixup-build branch March 1, 2024 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gorkem gorkem gorkem approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@amisevsk @gorkem