Skip to content
This repository was archived by the owner on Apr 23, 2026. It is now read-only.

feat: add ability to generate OCI artifact from local models #2977

Closed
jaideepr97 wants to merge 2 commits intoinstructlab:mainfrom
jaideepr97:add-oci
Closed

feat: add ability to generate OCI artifact from local models #2977
jaideepr97 wants to merge 2 commits intoinstructlab:mainfrom
jaideepr97:add-oci

Conversation

@jaideepr97
Copy link
Copy Markdown
Contributor

@jaideepr97 jaideepr97 commented Jan 23, 2025

This PR adds a new converter function to convert safetensors models to OCI artifact format before it can be pushed via ilab model upload

This PR builds on top of #2950

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the
    conventional commits.
  • Changelog updated with breaking and/or notable changes for the next minor release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Functional tests have been added, if necessary.
  • E2E Workflow tests have been added, if necessary.

@mergify mergify Bot added testing Relates to testing dependencies Relates to dependencies ci-failure PR has at least one CI failure labels Jan 23, 2025
@nathan-weinberg nathan-weinberg added the hold In-progress PR. Tag should be removed before merge. label Jan 23, 2025
@mergify mergify Bot added ci-failure PR has at least one CI failure and removed ci-failure PR has at least one CI failure labels Jan 23, 2025
@mergify mergify Bot added ci-failure PR has at least one CI failure and removed ci-failure PR has at least one CI failure labels Jan 23, 2025
@mergify mergify Bot added ci-failure PR has at least one CI failure and removed ci-failure PR has at least one CI failure labels Jan 23, 2025
@mergify mergify Bot added ci-failure PR has at least one CI failure and removed ci-failure PR has at least one CI failure labels Jan 26, 2025
@mergify mergify Bot added CI/CD Affects CI/CD configuration documentation Improvements or additions to documentation ci-failure PR has at least one CI failure and removed ci-failure PR has at least one CI failure labels Jan 26, 2025
@mergify mergify Bot added ci-failure PR has at least one CI failure and removed ci-failure PR has at least one CI failure labels Jan 27, 2025
@mergify mergify Bot removed the ci-failure PR has at least one CI failure label Jan 27, 2025
@booxter
Copy link
Copy Markdown
Contributor

booxter commented Jan 27, 2025

OK, about your mypy failures. tl;dr we'll have to ignore these (add type: ignore to affected lines).

  1. src/instructlab/configuration.py:204: error: Argument "default_factory" to "Field" has incompatible type "type[list[Any]]"; expected "Callable[[], Never] | Callable[[dict[str, Any]], Never]" [arg-type]

This is happening because of a bug in mypy that is triggered by update of pydantic to 2.10.x branch.

Your patch adds olot dependency that requires at least pydantic>=2.10.3, so we cannot consider capping pydantic to <2.10. We'll have to type: ignore the violation.

Please add the links to the two issues above when adding the ignores so we can track when we will be able to remove them.

  1. src/instructlab/configuration.py:1187: error: Too few arguments [call-arg]

This is happening because of a bug in pydantic. The bug is fixed in main branch but so far not released. We'll have to add type: ignore here too.

Please link to the issue and mention that pydantic>=2.10.7 will solve the issue.

I've also noticed that we don't enable mypy plugin for pydantic in the repo. I will send a patch to do this. It won't solve your problems though, so it's not related.

booxter
booxter reviewed Jan 27, 2025
View reviewed changes
Comment thread src/instructlab/configuration.py
@booxter booxter mentioned this pull request Jan 27, 2025
Merged
6 tasks
@mergify mergify Bot added the ci-failure PR has at least one CI failure label Jan 27, 2025
@mergify mergify Bot added ci-failure PR has at least one CI failure and removed ci-failure PR has at least one CI failure labels Jan 27, 2025
@mergify mergify Bot removed the ci-failure PR has at least one CI failure label Jan 27, 2025
@jaideepr97 @cdoern
rewrite ilab model convert
bc202f7 
Signed-off-by: Jaideep Rao <[email protected]>
Signed-off-by: Charlie Doern <[email protected]>
@mergify mergify Bot added the ci-failure PR has at least one CI failure label Jan 27, 2025
@jaideepr97
add oci converter
71b7a14 
Signed-off-by: Jaideep Rao <[email protected]>
@mergify mergify Bot added ci-failure PR has at least one CI failure and removed ci-failure PR has at least one CI failure labels Jan 27, 2025
@nathan-weinberg nathan-weinberg linked an issue Jan 28, 2025 that may be closed by this pull request
Add ability to convert models to OCI format directories #2787
Closed
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 29, 2025

This pull request has been automatically marked as stale because it has not had activity within 60 days. It will be automatically closed if no further activity occurs within 30 days.

@github-actions github-actions Bot added the stale label Mar 29, 2025
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Mar 29, 2025

This pull request has merge conflicts that must be resolved before it can be merged. @jaideepr97 please rebase it. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase This Pull Request needs to be rebased label Mar 29, 2025
@github-actions github-actions Bot removed the stale label Mar 30, 2025
mergify Bot added a commit that referenced this pull request Apr 28, 2025
@mergify
mypy: Enable pydantic plugin (#3012)
8d4b558 
This may help us avoid some spurious mypy warnings when it gets confused by pydantic magick.

https://docs.pydantic.dev/latest/integrations/mypy/

Noticed when looking at mypy violations under #2977









**Checklist:**

- [x] **Commit Message Formatting**: Commit titles and messages follow guidelines in the
  [conventional commits](https://www.conventionalcommits.org/en/v1.0.0/#summary).
- [ ] [Changelog](https://github.com/instructlab/instructlab/blob/main/CHANGELOG.md) updated with breaking and/or notable changes for the next minor release.
- [ ] Documentation has been updated, if necessary.
- [ ] Unit tests have been added, if necessary.
- [ ] Functional tests have been added, if necessary.
- [ ] E2E Workflow tests have been added, if necessary.



Approved-by: nathan-weinberg

Approved-by: RobotSail
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 29, 2025

This pull request has been automatically marked as stale because it has not had activity within 60 days. It will be automatically closed if no further activity occurs within 30 days.

@github-actions github-actions Bot added the stale label May 29, 2025
@tarilabs
Copy link
Copy Markdown
Contributor

tarilabs commented May 29, 2025

In my view, being able to push to OCI with Olot is a required capability, can't speak for the other refactor/update in this pr. Commenting for the Stale.

@github-actions github-actions Bot removed the stale label May 30, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 29, 2025

This pull request has been automatically marked as stale because it has not had activity within 60 days. It will be automatically closed if no further activity occurs within 30 days.

@github-actions github-actions Bot added the stale label Jul 29, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Aug 29, 2025

This pull request has been automatically closed due to inactivity. Please feel free to reopen if you intend to continue working on it!

@github-actions github-actions Bot closed this Aug 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@booxter booxter booxter left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CI/CD Affects CI/CD configuration ci-failure PR has at least one CI failure dependencies Relates to dependencies documentation Improvements or additions to documentation hold In-progress PR. Tag should be removed before merge. needs-rebase This Pull Request needs to be rebased stale testing Relates to testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add ability to convert models to OCI format directories

4 participants

@jaideepr97 @booxter @tarilabs @nathan-weinberg