Skip to content

disable instant queue mode on cloud #6141

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

Merged
merged 6 commits into from
Oct 20, 2025
Merged

disable instant queue mode on cloud #6141

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c495567
remove instant queue mode on cloud
christian-byrne Oct 19, 2025
ba2299e
Merge branch 'main' into disable-run-instant-on-cloud
christian-byrne Oct 20, 2025
c924695
[fix] add graceful fallback for unavailable queue modes - addresses r…
christian-byrne Oct 19, 2025
f7a96af
[refactor] improve type safety for labels with nullish coalescing - a…
christian-byrne Oct 19, 2025
940bc52
[test] add comprehensive test coverage for cloud-specific queue butto…
christian-byrne Oct 19, 2025
0d7b223
[test] remove problematic queue button tests - addresses review feedback
christian-byrne Oct 20, 2025
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
55 changes: 31 additions & 24 deletions src/components/actionbar/ComfyRunButton/ComfyQueueButton.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
showDelay: 600
}"
class="comfyui-queue-button"
:label="activeQueueModeMenuItem.label"
:label="String(activeQueueModeMenuItem.label)"
Copy link

Choose a reason for hiding this comment

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

[quality] medium Priority

Issue: Unnecessary type coercion with String() wrapper
Context: The labels from MenuItem objects should already be strings from i18n translations
Suggestion: Remove String() wrapper or add proper typing if there's a specific type issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Improved type safety with nullish coalescing in c761174

severity="primary"
size="small"
:model="queueModeMenuItems"
Expand Down Expand Up @@ -82,10 +82,12 @@
import { storeToRefs } from 'pinia'
import Button from 'primevue/button'
import ButtonGroup from 'primevue/buttongroup'
import type { MenuItem } from 'primevue/menuitem'
import SplitButton from 'primevue/splitbutton'
import { computed } from 'vue'
import { useI18n } from 'vue-i18n'

import { isCloud } from '@/platform/distribution/types'
import { useCommandStore } from '@/stores/commandStore'
import {
useQueuePendingTaskCountStore,
Expand All @@ -100,32 +102,37 @@ const queueCountStore = storeToRefs(useQueuePendingTaskCountStore())
const { mode: queueMode } = storeToRefs(useQueueSettingsStore())

const { t } = useI18n()
const queueModeMenuItemLookup = computed(() => ({
disabled: {
key: 'disabled',
label: t('menu.run'),
tooltip: t('menu.disabledTooltip'),
command: () => {
queueMode.value = 'disabled'
const queueModeMenuItemLookup = computed(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this computed?

const items: Record<string, MenuItem> = {
disabled: {
key: 'disabled',
label: t('menu.run'),
tooltip: t('menu.disabledTooltip'),
command: () => {
queueMode.value = 'disabled'
}
},
change: {
key: 'change',
label: `${t('menu.run')} (${t('menu.onChange')})`,
tooltip: t('menu.onChangeTooltip'),
command: () => {
queueMode.value = 'change'
}
}
},
instant: {
key: 'instant',
label: `${t('menu.run')} (${t('menu.instant')})`,
tooltip: t('menu.instantTooltip'),
command: () => {
queueMode.value = 'instant'
}
},
change: {
key: 'change',
label: `${t('menu.run')} (${t('menu.onChange')})`,
tooltip: t('menu.onChangeTooltip'),
command: () => {
queueMode.value = 'change'
}
if (!isCloud) {
items.instant = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Completely optional but there is a way to do this inline
Not sure if it's better, but you know I like avoiding object mutation.

key: 'instant',
label: `${t('menu.run')} (${t('menu.instant')})`,
tooltip: t('menu.instantTooltip'),
command: () => {
queueMode.value = 'instant'
}
}
}
}))
return items
})

const activeQueueModeMenuItem = computed(
Copy link

Choose a reason for hiding this comment

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

[architecture] high Priority

Issue: Missing runtime fallback handling for instant mode in cloud
Context: When removing the 'instant' menu item for cloud, existing users may still have queueMode='instant' stored in their settings
Suggestion: Add fallback logic: if (isCloud && queueMode.value === 'instant') queueMode.value = 'disabled'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added graceful fallback for unavailable queue modes in d297be3

() => queueModeMenuItemLookup.value[queueMode.value]
Expand Down