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

Give the possibility to set up configuration arguments in a file #4357

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Le09
Copy link

@Le09 Le09 commented Aug 14, 2024

I was looking up issue: #1292
Which felt a bit unsatisfying.
It turns out you can use configargparse as a drop-in replacement for argparse to get the feature for free.

Thank you very much for this amazing piece of software!

@ltdrdata
Copy link
Collaborator

ltdrdata commented Aug 15, 2024

You need to add configargparse into requirements.txt

@Le09
Copy link
Author

Le09 commented Aug 15, 2024

All right, thank you for catching this mistake, done!

@mcmonkey4eva mcmonkey4eva added User Support A user needs help with something, probably not a bug. Feature A new feature to add to ComfyUI. and removed User Support A user needs help with something, probably not a bug. labels Sep 12, 2024
@mcmonkey4eva
Copy link
Contributor

So, first: this is really neat and handy, and the core idea of having config files is definitely a good one

However: it will likely be a goal in the near future to give comfyui a "real server config", one that you can configure in UI on localhost frontend / apply edits at runtimes / etc. When that happens we'll potentially also looking at integrating together different configs (eg the extra_model_paths config) into one central server config.
So the issue with this PR would be, it adds a new comfyrc file, but that new file very quickly becomes a legacy file relative to the eventual "real config" file, which might end up making a bit of a mess

@Le09
Copy link
Author

Le09 commented Sep 19, 2024

@mcmonkey4eva thank you for your insight.
These are not mutually exclusive problems I think?
You just need to decide a file format, and a filename.
What I think might be limiting here is the .ini file format, for which you might prefer to work with Toml, Yaml, or Json?
As long as you can en/de-code, you can always have the server overwrite the file in the directory with the highest precedence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature A new feature to add to ComfyUI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants