8000 Separated user home folder virtual env by jakubjezek001 · Pull Request #10 · tweak-wtf/ayon-comfyui · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Separated user home folder virtual env #10

New issue
Jump to bottom
Open
wants to merge 3 commits into
base: enhancement/implement-ihostaddon
Choose a base branch
from
Open

Separated user home folder virtual env #10

wants to merge 3 commits into from

Conversation

jakubjezek001
Copy link
Collaborator
@jakubjezek001 jakubjezek001 commented Apr 4, 2025 •
edited
Loading

closes #9

@jakubjezek001
Add project name support and venv improvements
8541e10 
- Added project name to launch arguments.
- Enhanced virtual environment path handling based on project name.
- Updated activation script for the virtual environment.
- Ensured PyYAML is installed during setup.
@jakubjezek001 jakubjezek001 self-assigned this Apr 4, 2025
@jakubjezek001 jakubjezek001 requested a review from tweak-wtf April 4, 2025 09:07
@jakubjezek001 jakubjezek001 added the enhancement New feature or request label Apr 4, 2025
jakubjezek001
jakubjezek001 commented Apr 4, 2025
View reviewed changes
client/ayon_comfyui/tools/install_and_run_server_venv.ps1
Comment on lines 82 to 86
Write-Output "Running ComfyUI with CPU"
uv run .\main.py --cpu
python .\main.py --cpu
} else {
uv run .\main.py
python .\main.py
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this to be honest. I believe since the virtual env is activated this is starting python from the venv - so possibility it doesn't need to run it via uv, right?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

afaik uv run makes sure to that the python in the venv gets used.
since i'm activating the venv before doing anything else this should be totally fine.

tbh, i don't believe this is actually changing the behaviour

@tweak-wtf
Copy link
Owner

i'm changing the target branch as this is adressing a sub-issue of the larger "ComfyUI as AYON Application" issue which shall be closed by #12

@tweak-wtf tweak-wtf changed the base branch from develop to enhancement/implement-ihostaddon April 5, 2025 11:12
tweak-wtf
tweak-wtf reviewed Apr 5, 2025
View reviewed changes
Copy link
Owner
@tweak-wtf tweak-wtf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't think we need the project_name argument for the launch script.
i'm already launching the new shell session in the directory that was built using the configured folder templates here:

ayon-comfyui/client/ayon_comfyui/hooks/pre_launch.py

Line 108 in 138f51b

"cwd": self.comfy_root,

this already includes the project name so i believe this would be redundant.

@tweak-wtf
Copy link
Owner

okay so checking this out more i get a feeling that this is implementing behaviour that was already present just with more code.
if i understand the issue correctly it wants to make sure that venv is created and used from a user's local machine.
this is already possible.

the directory in which the launch script gets called is already built based on the repository_root_template in server settings which can point to local drives but that's completely up to the user. can also live in a shared location if so desired.
this PR however restricts users to only install comfy to the users $USERPROFILE. so it prevents users to e.g. install it to a local D drive or a different mount in UNIX.

the entire launch script operates in this folder context and therefore also uv. so a simple .venv/Scripts/activate is enough to get uv's python interpreter. it's also always found since the shell is already in the correct cwd

let's discuss :)

@tweak-wtf
Copy link
Owner
tweak-wtf commented Apr 5, 2025 •
edited
Loading

lemme share my approach.
i have configured a new root called local which points to D: for windows.
then i use that new root in repository_root_template like so:
{root[local]}/comfyui/{project[name]}

this causes comfy to always be installed at e.g. D:/comfyui/example_project no matter which task or folder context it was launched from.
probably not the nicest probably but it's configurable 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tweak-wtf tweak-wtf tweak-wtf left review comments

Assignees

@jakubjezek001 jakubjezek001

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

separated user home folder virtual env
2 participants
@jakubjezek001 @tweak-wtf
0