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

Moved logic of "post_prompt" endpoint to sub-function, allowed custom "prompt_id" #6129

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bigcat88
Copy link
Contributor

@bigcat88 bigcat88 commented Dec 20, 2024

Relates to #6114

The changes do not break backward compatibility, all logic remains the same.

Allows having the class PromptServer which is returned by the start_comfyui function from the linked PR, without opening a websocket connection, to add tasks for execution to the Comfy engine.

For everything to work together beautifully, there is missing ability to add custom own event_callback which will be called at the beginning of the PromptExecutor.add_message method, so that without connecting to the websocket you can receive all messages from the Comfy engine and monitor the progress (I already have this locally).

What do you say, @comfyanonymous, should I include this PR in the linked one and add the ability to call the callback in add_message, or will we merge both of these PRs, and I will open a third small one? (it depends on the first one because it changes the same files)

@bigcat88
Copy link
Contributor Author

If this is not interesting and there is no desire to merge this, then let me know and I will close both PRs

@bigcat88 bigcat88 marked this pull request as draft December 24, 2024 14:43
@bigcat88
Copy link
Contributor Author

I added the ability to specify a custom prompt_id when adding a task to the queue, this will be useful if you have a system that already has a task ID, and you can simply put a task with prompt_id=uniq_app_id-TASK_ID in Comfy, for example myapp-13 and then easily determine that this is your task with an ID of 13.

What is missing is the ability to specify your callback in server.PromptServer.send_sync and this will completely allow to use Comfy not through a default websocket(but still it will available and working), but from Python app directly - but it is better to do this in a separate PR.

And then we can move on to more serious things, if necessary.

@bigcat88 bigcat88 marked this pull request as ready for review December 24, 2024 17:03
@bigcat88
Copy link
Contributor Author

And I prefer this way of writing code, so that if necessary I can write in a style that is much shorter and that linters(ruff/pylint) won't complain about in the future if you planned to enable them:

# extra_data = {}
# if "extra_data" in json_data:
#    extra_data = json_data["extra_data"]
extra_data = json_data.get("extra_data", {})

if "client_id" in json_data:
    extra_data["client_id"] = json_data["client_id"]
if valid[0]:
    # if "prompt_id" in json_data:
    #     prompt_id = json_data["prompt_id"]
    # else:
    #     prompt_id = str(uuid.uuid4())        
    prompt_id = json_data.get("prompt_id", str(uuid.uuid4()))    

@bigcat88 bigcat88 changed the title Move logic of "post_prompt" endpoint to sub-function [refactor] Moved logic of "post_prompt" endpoint to sub-function, allowed custom "prompt_id" Dec 24, 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.

1 participant