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

Do not download model again #107

Merged
merged 6 commits into from
Jan 23, 2024
Merged

Do not download model again #107

merged 6 commits into from
Jan 23, 2024

Conversation

feloy
Copy link
Contributor

@feloy feloy commented Jan 22, 2024

Fixes #18

Copy link
Collaborator

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

would be nice (as for many others PR) to add more unit tests

@feloy feloy force-pushed the feat-18/dont-reload-model branch 2 times, most recently from 35f3046 to 9376725 Compare January 22, 2024 15:33
Comment on lines 18 to 49
// eslint-disable-next-line @typescript-eslint/no-explicit-any
vi.spyOn(fs, 'readdirSync').mockImplementation((dir: string): any => {
// TODO(feloy): fix any
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The difficulty with readdirSync is that it returns either Dirent[] or string[] depending on the options passed. I'm not sure how to handle this without using any here...

Copy link
Collaborator

@benoitf benoitf Jan 23, 2024

Choose a reason for hiding this comment

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

const readdirSyncMock: MockInstance<[path: fs.PathLike], string[]| fs.Dirent[]> = vi.spyOn(fs, 'readdirSync') as unknown as MockInstance<[path: fs.PathLike], string[]| fs.Dirent[]>;
  readdirSyncMock.mockImplementation((path: fs.PathLike) => {

Copy link
Collaborator

Choose a reason for hiding this comment

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

(that said on unit test I'm fine to use any type)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks

@feloy feloy force-pushed the feat-18/dont-reload-model branch 2 times, most recently from 6a0c083 to 8f39872 Compare January 22, 2024 16:21
@feloy feloy changed the title Do not download model again [wip] Do not download model again Jan 22, 2024
@feloy feloy force-pushed the feat-18/dont-reload-model branch 3 times, most recently from eec5015 to ccb3533 Compare January 23, 2024 07:20
@feloy feloy force-pushed the feat-18/dont-reload-model branch from ccb3533 to 0224f4f Compare January 23, 2024 07:22
@feloy feloy changed the title [wip] Do not download model again Do not download model again Jan 23, 2024
@feloy feloy merged commit 51d9a65 into main Jan 23, 2024
3 checks passed
@feloy feloy deleted the feat-18/dont-reload-model branch January 23, 2024 08:15
mhdawson pushed a commit to mhdawson/podman-desktop-extension-ai-lab that referenced this pull request Nov 22, 2024
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.

fix: Do not download models already on disk
2 participants