404background / node-red-contrib-python-venv

Node for python virtual environment
https://flows.nodered.org/node/@background404/node-red-contrib-python-venv
MIT License
10 stars 4 forks source link

Venv naming with space fix #17

Closed FrederikPM closed 4 months ago

FrederikPM commented 4 months ago

Fixed venv names with spaces breaking. Additonally added a check for venv existing such that the nodes will not function before a venv is selected. In addition also added "default" to the version displayed when entering the venv configuration for the first time to make it clearer that it is an option.

404background commented 4 months ago

Thank you @FrederikPM ! I intended to run the node without setting venv-config for backward compatibility, but if I get an error that it is not set, I think it is fine even if I can't run it.

I have checked that the version is set to "default". image

In a previous Pull Request #6 , there was a case that I could not execute due to a difference in the quotation type. In this case, the same error occurred in the Windows environment.

Single quotes were treated as part of the string. image

Double quotes resulted in the expected folder name. I also checked that it can be created even if there is a space in the name. image

Also, if "default" is entered in this.version in venv-config.js, the virtual environment cannot be created. This is because when it should be -3.8, etc., it becomes -default like below. python -default -m venv pyenv

If "default" is not set in this.version in venv-config.js, the virtual environment can be created. image

The modified program for venv-config.js is as follows. Could you try it in your environment ?

``` module.exports = function(RED) { function venvConfig(config) { RED.nodes.createNode(this, config) this.venvname = config.venvname this.version = config.version const path = require('path') const fs = require('fs') const execSync = require('child_process').execSync const setupPath = path.join(path.dirname(__dirname), 'setup.py') let venvPath = path.join(path.dirname(__dirname), this.venvname) let command = `python ${setupPath} "${this.venvname}"` if(typeof this.version !== 'undefined' && this.version !== '' && this.version !== 'default') { command += ` ${this.version}` } execSync(command) this.on('close', function(removed, done) { if (removed) { fs.rmSync(venvPath, { recursive: true, force: true }) } done() }) } RED.nodes.registerType("venv-config",venvConfig) } ```
FrederikPM commented 4 months ago

Ah I didn't notice how the default environment was actually parsed to a version. I've added your changes to the PR :)

404background commented 4 months ago

Thank you @FrederikPM ! I confirmed that it works correctly.

I merge this Pull Request, and modify README.