invoke-ai
Non-interactive model download (support `HUGGINGFACE_TOKEN`)
This PR adds the following improvements:
Support for the HUGGINGFACE_TOKEN environment variable
Ensures that the SD weights download proceeds in a truly non-interactive fashion when the --yes flag is used and a valid authentication token is present in any of the supported locations (env vars or cached).
Tested as follows:
HUGGING_FACE_HUB_TOKENenv var is not empty:- with
--yes: weights download proceeds; token is not persisted in cache - without
--yes: weights download proceeds without an additional prompt; token is not persisted in cache
- with
HUGGINGFACE_TOKENenv var is not empty:- same behaviour as with the official var (token not persisted)
- Env vars not present without
--yesflag: user is prompted for the token, token is cached only if valid (same as current behaviour) - Env vars not present with
--yesflag: weights download is skipped (same as current behaviour) - In any case where the token is invalid (i.e. login to HF does not succeed), the token will not be cached.
Additionally:
- fixes a bug where
outdirlocation was not printed due to missing f-string - fixes spelling of
--outdirargument in a message - implements a way of exposing download failures at the end of configuration run. if model download failed or was skipped, the postscript message was misleadingly suggesting that the application was ready to use.
huggingface_hub.login(token) could be a more-explicit alternative to resetting os.environ inside the script.
Also note that they've changed the settings on some of the repos so now not all of the Stable Diffusion models require a token. As of last time I checked earlier this weekend, the CompVis/ ones don't (SD 1.4) but the runwayml/ ones do (SD 1.5 + SD 1.5 inpainting).
huggingface_hub.login(token)could be a more-explicit alternative to resettingos.environinside the script.
That is how I initially implemented it, but the behaviour becomes inconsistent between using the officially supported env var HUGGING_FACE_HUB_TOKEN and the de-facto "community preferred" HUGGINGFACE_TOKEN. Calling the .login() method causes the token to be saved in ~/.huggingface/token, whereas using the official env. var does not.
Moving the initialization file is proving tricky if we want to maintain backwards compatibility with the current setup, because we want to move it into the runtime dir, but users might already have a conflicting --root-dir argument in an existing ~/.invokeai file. Also, setting it from Globals before checking for a user-specified runtime dir creates a catch-22.
TLDR; Perhaps the initfile should be dealt with in a separate PR so that this one isn't blocked for too long. Unless we decide to break backwards-compatibility, then it's easy
Moving the initialization file is proving tricky if we want to maintain backwards compatibility with the current setup, because we want to move it into the runtime dir, but users might already have a conflicting
--root-dirargument in an existing~/.invokeaifile. Also, setting it fromGlobalsbefore checking for a user-specified runtime dir creates a catch-22.TLDR; Perhaps the initfile should be dealt with in a separate PR so that this one isn't blocked for too long. Unless we decide to break backwards-compatibility, then it's easy
Why not have an environment variable that contains the path to the init file?
As an aside, introducing Globals wasn't my favorite approach, but it was better than carrying a directory path across multiple nested functions.
Just to set expectations, unless the release gets delayed significantly I'm going to go with the existing configuration script for 2.2.0, and release the new and improved one that you're working on for 2.2.1. This is primarily because I've got a lot of documentation-writing to do and won't be able to do thorough testing on this. I very much appreciate your work on this bit of the code.
I also have a suggestion for a new feature. Either as a command-line option or as an automatic feature during interactive processing, it would be great if the script could scan the existing models directory for SD models it knows about (from INITIAL_MODELS.yaml). If it finds SD models that aren't in the config file, it should offer to add them. This will let people rebuild their config.yaml without having to ask the script to download the models and answer 'y' to each prompt.
That being said, if I do have time I will try very hard to get this in. The improvements look very good.
All sounds great to me. I actually might have a viable approach to handling the .invokeai config file location in a backwards-compatible manner, but certainly wouldn't want any of this to delay the release. If anything, that would give me more time to do some more refactoring and properly test the changes. But I'm aiming try to get the final fixes in tonight.
I think I was able to wrangle the config file location into a good state; this is now ready for review.
@lstein would you like me to tackle the auto-scanning of models in this same PR, or open a new feature request for it? I'd prefer the latter as it might get messy to review otherwise, but it's your call, please let me know.
I think I was able to wrangle the config file location into a good state; this is now ready for review.
@lstein would you like me to tackle the auto-scanning of models in this same PR, or open a new feature request for it? I'd prefer the latter as it might get messy to review otherwise, but it's your call, please let me know.
Go ahead and start a new PR. I'm going to integrate what you've done here with a few minor changes I made on a different branch and then merge.
I seem to have oddly broken some tests, let me look into that :thinking:
Edit: I can't reproduce these test failures locally. On closer look, this might be a transient failure: requests.exceptions.HTTPError: 504 Server Error: Gateway Time-out for url: https://huggingface.co/bert-base-uncased/resolve/main/tokenizer.json, but the configuration script exits without failure and so the application is left in a not-fully-configured state: OSError: Can't load tokenizer for 'openai/clip-vit-large-patch14'
I started testing and then realized that the default location for the init file has become the InvokeAI repo.
That sounds unexpected and not intentional. The logic I was trying to implement was to: 1. if an existing config file is found in the user's home directory, then keep using it, and 2. if no existing file is found, then create one in the runtime directory.
If the default location of the runtime directory happens to be the root of the repo, then yes, this is a problem. I think this stems from:
# This is usually overwritten by the command line and/or environment variables
- Globals.root = '.'
+ Globals.root = root_dir if (root_dir := os.getenv("INVOKEAI_ROOT")) else "."
But this behaviour should have remained unchanged if neither the env var nor the --root_dir are specified. I.e. Globals.root was already "." before this PR.
Agreed that we never want to mix the runtime dir and neither the cloned codebase nor especially the dev-installed Python module, as you mention. Would it make sense to instead default the runtime dir to be located at ~/invokeai/ instead of the more ambiguous "."?
This is really tricky to finesse because the init file tells invoke where the root directory is, but if the init file is inside the root directory how does invoke find it?
Definitely, that's the catch-22 situation I was talking about earlier. I think the best solution is to remove ambiguity by having a sane default that's more explicit and guaranteed to be outside of the codebase.
Yeah, I think we need to reverse the logic and have a predictable location of the invokeai directory rather than a predictable init file. The init file can then live inside invokeai.
I also just noticed that the configure_invokeai.py, the initfile comments, and the CI tests all specify the --root option, while in args.py, CLI.py and elsewhere in the app we have --root_dir.
@lstein perhaps leaving this PR unmerged until after 2.2.0 is indeed for the best, so that we can clean it all up after the release dust settles. I will keep rebasing+updating it though. Do you agree?
(CLI argument handling and various entrypoints could use quite a bit of future refactoring + consolidation in general, IMHO)
I moved the init file / root dir changes to another branch and will do a separate PR for it, as it was getting quite messy and needs a larger refactor.
This is ready to be reviewed again, because the runtime dir / init file location refactoring will be dealt with separately, likely in https://github.com/invoke-ai/InvokeAI/pull/1615
