-
Notifications
You must be signed in to change notification settings - Fork 53
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
Isolate dependencies from other add-ons #284
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a nice improvement. I have a few questions though:
- Why have the install/uninstall deps operators been changed to only work with a single module?
- Why did you create a dependencies folder with an
__init__.py
instead of just creating dependencies.py? - Why has the check to ensure pip is present been removed from the install deps operator?
Oh, also, I think this will leave the old selenium module installed. It would be nice to remove it, if that's not too hard. |
Yeah that's a problem with the way we were doing things before we can't really remove it as we don't know if any other add-on might have installed or is using it. |
Thank you for the updates.
Good point. That makes sense. Any update on my questions from above?
|
We were only using these operators to install one dependency so I simplified the code to just get one dep.
No strong reason, I guess it makes it simple to do
I think we can safely assume that pip is always present in Blender installs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We were only using these operators to install one dependency so I simplified the code to just get one dep.
I figured it might be nice to have one button to install all dependencies and that supporting multiple would future proof that, but one button per dependency works too and is simpler. So, I guess I don't have a strong opinion either way.
No strong reason, I guess it makes it simple to do from .dependencies import get_XXX
I believe you can do that with a regular .py file, but this works fine too; just seems a little weird to me, that's all.
I think we can safely assume that pip is always present in Blender installs
I just tried with Blender-3.3.2 on my system and got this error:
01 Dependencies install has failed
02 /home/<user>/Workshop/CustomApps/blender-3.3.2/3.3/python/bin/python3.10: No module named pip
So I think ensure pip needs to remain. Some people recommend upgrading pip as well, but I'm not entirely sure if we need to: https://blender.stackexchange.com/questions/56011/how-to-install-pip-for-blenders-bundled-python
I've addressed the feedback and added back pip and added pip upgrade. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
This PR isolates selenium from the Blender python packages to avoid conflicts. We still do it using pip but using a specific target directory and now the modules are loaded in isolation using importlib.