Major Issue with SD.Next

[Aptronymist]

Due to your code importing the entirety of modules.scripts (starting in /io/util.py line 6, but perhaps other places too), it is creating a circular import by importing our extensions.py.

Vlad has informed me that (paraphrasing) "this is a big no no, blindly loading scripts or extensions should never ever be done."

from my log:

I personally love your extension, as do many users, so please let me know what we can work out together to address this, if you need any assistance at all, Vlad is always happy to do so. You can join our discord server if you wish, extension devs are always welcome and have a role, or simply visit our repo, however you like.

Thank you for your wonderfully useful extension and I look forward to cooperating to get things going again!

[vladmandic]

just to be clear, the issue is not importing modules.scripts or modules.shared in general, the issue is that it does that in install.py which runs BEFORE server has started. ONLY allowed import from core app in install.py is launch.py to get access to pip installer.

importing scripts or shared from installer basically makes app run inside extensions instead of extension running inside app.

[ototadana]

@Aptronymist Thanks for using this extension.

Perhaps I can stop using modules.scripts, but I can't think of a good way to do it for modules.shared.

The extension's installer uses the Additional components setting to select what to install in order to avoid installing unnecessary dependencies, but if the installer is forbidden to use modules.shared, this feature will not work properly. One possible workaround is to have the installer read the configuration directly from the config.json file, but this is an imperfect solution since the configuration may not be what the config.json user intended (specified by command line options). Also, it is possible that the config.json specification may change in the future.

[Aptronymist]

@Aptronymist Thanks for using this extension.

Perhaps I can stop using modules.scripts, but I can't think of a good way to do it for modules.shared.

The extension's installer uses the Additional components setting to select what to install in order to avoid installing unnecessary dependencies, but if the installer is forbidden to use modules.shared, this feature will not work properly. One possible workaround is to have the installer read the configuration directly from the config.json file, but this is an imperfect solution since the configuration may not be what the config.json user intended (specified by command line options). Also, it is possible that the config.json specification may change in the future.

Best to ignore what I said and focus entirely on what @vladmandic said, I was just coming to get the ball rolling on the situation with an incomplete understanding of the cause.

What he said is the actual issue, and that's what should be addressed. It's just the install.py that needs fixing.

[ototadana]

@Aptronymist Yes. Actually I found a way around the error you encountered. However, I find it quite difficult to correct it in the way it should be, for the reasons I mentioned above.

[vladmandic]

@Aptronymist Thanks for using this extension.

Perhaps I can stop using modules.scripts, but I can't think of a good way to do it for modules.shared.

The extension's installer uses the Additional components setting to select what to install in order to avoid installing unnecessary dependencies, but if the installer is forbidden to use modules.shared, this feature will not work properly. One possible workaround is to have the installer read the configuration directly from the config.json file, but this is an imperfect solution since the configuration may not be what the config.json user intended (specified by command line options). Also, it is possible that the config.json specification may change in the future.

install.py is for critical dependencies. if you want to have optional components depending on user settings, you can install them in runtime during main extension execution, not in install.py.

note that even server does not have opts parsed at all at the time when install.py is triggered. so you're basically starting a server to read opts inside extension - a total mess.

[ototadana]

if you want to have optional components depending on user settings, you can install them in runtime during main extension execution, not in install.py.

Thanks, I'll see if I can handle it that way.

[ototadana]

@Aptronymist Modified installation process for SD.Next, available on the sd.next branch. Please try it out.

[Aptronymist]

@Aptronymist Modified installation process for SD.Next, available on the sd.next branch. Please try it out.

I did! Worked just fine!

Thank you for your assistance, believe me, you saved us both from having to deal with issues and complaints from users. That's why I wanted to get out ahead of it before our dev branch merges in.

[Aptronymist]

Oh wait, I should have asked, when will this go through? I think we're due for the merge tomorrow, and while I know I'll be busy with support and bugs, but I don't want people to be bugging you.

[ototadana]

I am sorry. I forgot to explain.

I don't intend to merge the sd.next branch into the main branch, there is much incompatibility between SD.Next and Automatic1111. So there is a risk that a modification for one software will not work for the other. To avoid this, I believe it is better to maintain each on its own branch.

I will include this information in the README later.

[vladmandic]

i don't understand - the best practises how to write well behaved extension are the same for a1111 and sdnext - and this issue is absolutely valid in both. the difference is that sdnext is stricter, but that's it.

[ototadana]

Yes, regarding this issue, I completely agree with what you're saying.

However, there's always a risk of incompatibility with future changes. For instance, even if we merge the sd.next branch into the main branch now, it won't work properly in SD.Next. That's because the latest commit in the main branch is incompatible with SD.Next. Of course, I believe this incompatibility can be resolved. But that will take time. I don't want to spend time worrying about compatibility between the two software every time I make an update or modification, and testing in both environments.

By separating the branches, I won't have to worry about updates for one software ruining the functionality on the other.

[vladmandic]

totally your choice. my experience is that by separating branches, you end up with much higher maintenance overhead than actively maintaining single branch. but thats a philosophical question, my comment on this particular issue remains - there is correct and incorrect way of doing things and importing core app modules from installer should never be done.

[ototadana]

Thanks for the advice 😄