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

initial download-folder-selector interface #890

Merged
merged 24 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
806cad6
initial download-folder-selector interface
mcmonkey4eva Sep 19, 2024
e712e8e
use primevue select
mcmonkey4eva Sep 19, 2024
1e6a66c
add a folder select visibility checkbox
mcmonkey4eva Sep 19, 2024
06f03cc
slightly reduce indirection
mcmonkey4eva Sep 19, 2024
2b0d412
Merge branch 'main' into downloader-folder-selection
mcmonkey4eva Sep 22, 2024
a9b64e9
fix up select box updating
mcmonkey4eva Sep 22, 2024
ac9fa5d
Merge remote-tracking branch 'origin/dev1.3' into downloader-folder-s…
mcmonkey4eva Sep 23, 2024
a6ed52f
revert bad upstream changes
mcmonkey4eva Sep 23, 2024
36ddd7c
cleanup
mcmonkey4eva Sep 23, 2024
a701832
allow localhost sourced models in ui side
mcmonkey4eva Sep 23, 2024
0cc48d3
add screenshot expectations to test
mcmonkey4eva Sep 23, 2024
980540c
Merge branch 'dev1.3' into downloader-folder-selection
mcmonkey4eva Sep 23, 2024
be0fab9
Update test expectations [skip ci]
invalid-email-address Sep 23, 2024
557f81a
add testing of folder select
mcmonkey4eva Sep 23, 2024
e1e8c88
fix test
mcmonkey4eva Sep 23, 2024
e84366f
don't exclude folder selector when there's only 1
mcmonkey4eva Sep 23, 2024
ae3f927
oo - fix checkbox
mcmonkey4eva Sep 23, 2024
6190727
Update test expectations [skip ci]
invalid-email-address Sep 23, 2024
6f9b5bf
testing - don't expect screenshots :(
mcmonkey4eva Sep 24, 2024
8b7da47
experimental new test code
mcmonkey4eva Sep 24, 2024
5456fa3
toHaveClass is silly
mcmonkey4eva Sep 24, 2024
500c9b1
Merge branch 'dev1.3' into downloader-folder-selection
mcmonkey4eva Sep 24, 2024
3c96790
Merge branch 'dev1.3' into downloader-folder-selection
mcmonkey4eva Sep 24, 2024
048e711
add // comments documenting intent of allowedSources
mcmonkey4eva Sep 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 27 additions & 11 deletions browser_tests/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,37 +45,53 @@ test.describe('Execution error', () => {

test.describe('Missing models warning', () => {
test.beforeEach(async ({ comfyPage }) => {
await comfyPage.setSetting('Comfy.Workflow.ShowMissingModelsWarning', true)
await comfyPage.page.evaluate((url: string) => {
return fetch(`${url}/api/devtools/cleanup_fake_model`)
}, comfyPage.url)
await comfyPage.setSetting('Comfy.Workflow.ModelDownload.AllowedSources', [
'http://localhost:8188'
])
await comfyPage.setSetting('Comfy.Workflow.ModelDownload.AllowedSuffixes', [
'.safetensors'
])
})

test('Should display a warning when missing models are found', async ({
comfyPage
}) => {
await comfyPage.setSetting('Comfy.Workflow.ShowMissingModelsWarning', true)

// The fake_model.safetensors is served by
// https://github.com/Comfy-Org/ComfyUI_devtools/blob/main/__init__.py
await comfyPage.loadWorkflow('missing_models')

// Wait for the element with the .comfy-missing-models selector to be visible
const missingModelsWarning = comfyPage.page.locator('.comfy-missing-models')
await expect(missingModelsWarning).toBeVisible()

// Click the download button
const downloadButton = comfyPage.page.getByLabel('Download')
await expect(downloadButton).toBeVisible()
await downloadButton.click()

// Wait for the element with the .download-complete selector to be visible
const downloadComplete = comfyPage.page.locator('.download-complete')
await expect(downloadComplete).toBeVisible()
})

test('Can configure download folder', async ({ comfyPage }) => {
await comfyPage.loadWorkflow('missing_models')

const missingModelsWarning = comfyPage.page.locator('.comfy-missing-models')
await expect(missingModelsWarning).toBeVisible()

const folderSelectToggle = comfyPage.page.locator(
'.model-path-select-checkbox'
)
const folderSelect = comfyPage.page.locator('.model-path-select')
await expect(folderSelectToggle).toBeVisible()
await expect(folderSelect).not.toBeVisible()

await folderSelectToggle.click() // show the selectors
await expect(folderSelect).toBeVisible()

await folderSelect.click() // open dropdown
await expect(folderSelect).toHaveClass(/p-select-open/)

await folderSelect.click() // close the dropdown
await expect(folderSelect).not.toHaveClass(/p-select-open/)

await folderSelectToggle.click() // hide the selectors
await expect(folderSelect).not.toBeVisible()
})
})
113 changes: 88 additions & 25 deletions src/components/dialog/content/MissingModelsWarning.vue
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@
<p class="warning-description">
When loading the graph, the following models were not found:
</p>
<p class="warning-options">
<Checkbox
class="model-path-select-checkbox"
v-model="showFolderSelect"
label="Show folder selector"
:binary="true"
/>
Show folder selector
</p>
<ListBox
:options="missingModels"
optionLabel="label"
Expand All @@ -29,6 +38,19 @@
</div>
</div>
<div class="model-action">
<Select
class="model-path-select"
v-if="
slotProps.option.action &&
!slotProps.option.downloading &&
!slotProps.option.completed &&
!slotProps.option.error &&
showFolderSelect
"
v-model="slotProps.option.folderPath"
:options="slotProps.option.paths"
@change="updateFolderPath(slotProps.option, $event)"
/>
<Button
v-if="
slotProps.option.action &&
Expand Down Expand Up @@ -60,19 +82,22 @@

<script setup lang="ts">
import { ref, computed } from 'vue'
import Checkbox from 'primevue/checkbox'
import ListBox from 'primevue/listbox'
import Select from 'primevue/select'
import { SelectChangeEvent } from 'primevue/select'
import Button from 'primevue/button'
import { api } from '@/scripts/api'
import { DownloadModelStatus } from '@/types/apiTypes'
import { useSettingStore } from '@/stores/settingStore'

const settingStore = useSettingStore()
const allowedSources = settingStore.get(
'Comfy.Workflow.ModelDownload.AllowedSources'
)
const allowedSuffixes = settingStore.get(
'Comfy.Workflow.ModelDownload.AllowedSuffixes'
)
const showFolderSelect = ref(false)

Copy link
Member

Choose a reason for hiding this comment

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

Why this becomes hard coded again? We do want to expose this configurability to cloud deployment platforms to download from custom endpoints.

const allowedSources = [
'https://civitai.com/',
'https://huggingface.co/',
'http://localhost:'
mcmonkey4eva marked this conversation as resolved.
Show resolved Hide resolved
]
const allowedSuffixes = ['.safetensors', '.sft']

interface ModelInfo {
name: string
Expand All @@ -83,19 +108,26 @@ interface ModelInfo {
completed?: boolean
progress?: number
error?: string
folder_path?: string
}

const props = defineProps<{
missingModels: ModelInfo[]
paths: Record<string, string[]>
maximized: boolean
}>()

const modelDownloads = ref<Record<string, ModelInfo>>({})
let lastModel: string | null = null

const updateFolderPath = (model: any, event: SelectChangeEvent) => {
const downloadInfo = modelDownloads.value[model.name]
downloadInfo.folder_path = event.value
return false
}
const handleDownloadProgress = (detail: DownloadModelStatus) => {
if (detail.download_path) {
lastModel = detail.download_path.split('/', 2)[1]
lastModel = detail.download_path
}
if (!lastModel) return
if (detail.status === 'in_progress') {
Expand Down Expand Up @@ -134,7 +166,8 @@ const handleDownloadProgress = (detail: DownloadModelStatus) => {
const triggerDownload = async (
url: string,
directory: string,
filename: string
filename: string,
folder_path: string
) => {
modelDownloads.value[filename] = {
name: filename,
Expand All @@ -143,49 +176,75 @@ const triggerDownload = async (
downloading: true,
progress: 0
}
const download = await api.internalDownloadModel(url, directory, filename, 1)
const download = await api.internalDownloadModel(
url,
directory,
filename,
1,
folder_path
)
lastModel = filename
handleDownloadProgress(download)
}

api.addEventListener('download_progress', (event) => {
api.addEventListener('download_progress', (event: CustomEvent) => {
handleDownloadProgress(event.detail)
})

const missingModels = computed(() => {
return props.missingModels.map((model) => {
const downloadInfo = modelDownloads.value[model.name]
if (!allowedSources.some((source) => model.url.startsWith(source))) {
const paths = props.paths[model.directory]
if (model.directory_invalid || !paths) {
return {
label: `${model.directory} / ${model.name}`,
hint: model.url,
error:
'Download not allowed from this source: ' + allowedSources.join(', ')
error: 'Invalid directory specified (does this require custom nodes?)'
}
}
if (!allowedSuffixes.some((suffix) => model.name.endsWith(suffix))) {
const downloadInfo: ModelInfo = modelDownloads.value[model.name] ?? {
downloading: false,
completed: false,
progress: 0,
error: null,
name: model.name,
directory: model.directory,
url: model.url,
folder_path: paths[0]
}
modelDownloads.value[model.name] = downloadInfo
if (!allowedSources.some((source) => model.url.startsWith(source))) {
return {
label: `${model.directory} / ${model.name}`,
hint: model.url,
error: 'Only allowed suffixes are ' + allowedSuffixes.join(', ')
error: `Download not allowed from source '${model.url}', only allowed from '${allowedSources.join("', '")}'`
}
}
if (model.directory_invalid) {
if (!allowedSuffixes.some((suffix) => model.name.endsWith(suffix))) {
return {
label: `${model.directory} / ${model.name}`,
hint: model.url,
error: 'Invalid directory specified (does this require custom nodes?)'
error: `Only allowed suffixes are: '${allowedSuffixes.join("', '")}'`
}
}
return {
label: `${model.directory} / ${model.name}`,
hint: model.url,
downloading: downloadInfo?.downloading ?? false,
completed: downloadInfo?.completed ?? false,
progress: downloadInfo?.progress ?? 0,
error: downloadInfo?.error,
downloading: downloadInfo.downloading,
completed: downloadInfo.completed,
progress: downloadInfo.progress,
error: downloadInfo.error,
name: model.name,
paths: paths,
folderPath: downloadInfo.folder_path,
action: {
text: 'Download',
callback: () => triggerDownload(model.url, model.directory, model.name)
callback: () =>
triggerDownload(
model.url,
model.directory,
model.name,
downloadInfo.folder_path
)
}
}
})
Expand Down Expand Up @@ -218,6 +277,10 @@ const missingModels = computed(() => {
margin-bottom: 1rem;
}

.warning-options {
color: var(--fg-color);
}

.missing-models-list {
max-height: 300px;
overflow-y: auto;
Expand Down
10 changes: 8 additions & 2 deletions src/scripts/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,8 @@ class ComfyApi extends EventTarget {
url: string,
model_directory: string,
model_filename: string,
progress_interval: number
progress_interval: number,
folder_path: string
): Promise<DownloadModelStatus> {
const res = await this.fetchApi('/internal/models/download', {
method: 'POST',
Expand All @@ -378,7 +379,8 @@ class ComfyApi extends EventTarget {
url,
model_directory,
model_filename,
progress_interval
progress_interval,
folder_path
})
})
return await res.json()
Expand Down Expand Up @@ -687,6 +689,10 @@ class ComfyApi extends EventTarget {
async getLogs(): Promise<string> {
return (await axios.get(this.internalURL('/logs'))).data
}

async getFolderPaths(): Promise<Record<string, string[]>> {
return (await axios.get(this.internalURL('/folder_paths'))).data
}
}

export const api = new ComfyApi()
6 changes: 4 additions & 2 deletions src/scripts/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2178,10 +2178,11 @@ export class ComfyApp {
})
}

showMissingModelsError(missingModels) {
showMissingModelsError(missingModels, paths) {
if (useSettingStore().get('Comfy.Workflow.ShowMissingModelsWarning')) {
showMissingModelsWarning({
missingModels,
paths,
maximizable: true
})
}
Expand Down Expand Up @@ -2403,7 +2404,8 @@ export class ComfyApp {
this.showMissingNodesError(missingNodeTypes)
}
if (missingModels.length && showMissingModelsDialog) {
this.showMissingModelsError(missingModels)
const paths = await api.getFolderPaths()
this.showMissingModelsError(missingModels, paths)
}
await this.#invokeExtensionsAsync('afterConfigureGraph', missingNodeTypes)
requestAnimationFrame(() => {
Expand Down
1 change: 1 addition & 0 deletions src/services/dialogService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export function showLoadWorkflowWarning(props: {

export function showMissingModelsWarning(props: {
missingModels: any[]
paths: Record<string, string[]>
[key: string]: any
}) {
const dialogStore = useDialogStore()
Expand Down
12 changes: 0 additions & 12 deletions src/stores/coreSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,18 +203,6 @@ export const CORE_SETTINGS: SettingParams[] = [
type: 'hidden',
defaultValue: 'cover'
},
{
id: 'Comfy.Workflow.ModelDownload.AllowedSources',
name: 'Allowed model download sources',
type: 'hidden',
defaultValue: ['https://huggingface.co/', 'https://civitai.com/']
},
{
id: 'Comfy.Workflow.ModelDownload.AllowedSuffixes',
name: 'Allowed model download suffixes',
type: 'hidden',
defaultValue: ['.safetensors', '.sft']
},
{
id: 'Comfy.GroupSelectedNodes.Padding',
name: 'Group selected nodes padding',
Expand Down
2 changes: 0 additions & 2 deletions src/types/apiTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -498,8 +498,6 @@ const zSettings = z.record(z.any()).and(
'Comfy.Validation.Workflows': z.boolean(),
'Comfy.Workflow.SortNodeIdOnSave': z.boolean(),
'Comfy.Queue.ImageFit': z.enum(['contain', 'cover']),
'Comfy.Workflow.ModelDownload.AllowedSources': z.array(z.string()),
'Comfy.Workflow.ModelDownload.AllowedSuffixes': z.array(z.string()),
'Comfy.Node.DoubleClickTitleToEdit': z.boolean(),
'Comfy.Window.UnloadConfirmation': z.boolean(),
'Comfy.NodeBadge.NodeSourceBadgeMode': zNodeBadgeMode,
Expand Down
Loading