Skip to content

Conversation

@jjw24
Copy link
Member

@jjw24 jjw24 commented Jun 17, 2021

switch to using python 3.8.9 embeddable

  1. prompt user if want to manually set python path or use Python Embeddable 3.8.9
  2. if user chooses manually set, dialog box opens for selecting the folder
  3. if chooses to let flow install python flow will download and extract the Python Embeddable and set it as the python path for use on python plugins

@jjw24 jjw24 added the enhancement New feature or request label Jun 17, 2021
@jjw24 jjw24 self-assigned this Jun 17, 2021
@jjw24 jjw24 marked this pull request as draft June 17, 2021 12:21
@jjw24
Copy link
Member Author

jjw24 commented Jun 18, 2021

@taooceros @Zeroto521 there is no need to use where.exe or search env PATH to find the python path right? Either set it to the embeddable python executable or ask the users to point it to a python executable themselves.

var whereProcess = Process.Start(new ProcessStartInfo
{
FileName = "where.exe",
Arguments = "pythonw",
RedirectStandardOutput = true,
CreateNoWindow = true,
UseShellExecute = false
});
var pythonPath = whereProcess?.StandardOutput.ReadToEnd().Trim();
if (!string.IsNullOrEmpty(pythonPath))
{
pythonPath = FilesFolders.GetPreviousExistingDirectory(FilesFolders.LocationExists, pythonPath);
}
if (string.IsNullOrEmpty(pythonPath))
{
var paths = Environment.GetEnvironmentVariable(PATH);
pythonPath = paths?
.Split(';')
.FirstOrDefault(p => p.ToLower().Contains(Python));
}

@taooceros
Copy link
Member

@taooceros @Zeroto521 there is no need to use where.exe or search env PATH to find the python path right? Either set it to the embeddable python executable or ask the users to point it to a python executable themselves.

var whereProcess = Process.Start(new ProcessStartInfo
{
FileName = "where.exe",
Arguments = "pythonw",
RedirectStandardOutput = true,
CreateNoWindow = true,
UseShellExecute = false
});
var pythonPath = whereProcess?.StandardOutput.ReadToEnd().Trim();
if (!string.IsNullOrEmpty(pythonPath))
{
pythonPath = FilesFolders.GetPreviousExistingDirectory(FilesFolders.LocationExists, pythonPath);
}
if (string.IsNullOrEmpty(pythonPath))
{
var paths = Environment.GetEnvironmentVariable(PATH);
pythonPath = paths?
.Split(';')
.FirstOrDefault(p => p.ToLower().Contains(Python));
}

We can keep it, unless portable is set. If portable is set, I think it would be better to let user choose whether to use the embeddable python.

@jjw24
Copy link
Member Author

jjw24 commented Jun 18, 2021

This doesn't prompt user at all, just silently assigns if paths has python. I think we remove it so it prompts user to download portable or chooses to set it themselves. Essentially just two options

@taooceros
Copy link
Member

This doesn't prompt user at all, just silently assigns if paths has python. I think we remove it so it prompts user to download portable or chooses to set it themselves. Essentially just two options

ok, got you. Maybe we can have another option to detect python path automatically?

@jjw24
Copy link
Member Author

jjw24 commented Jun 18, 2021

From a quick glance It's just grabbing the python path from PATH, there is no need to do this though, because if the user chooses to set it themselves they will know where to set it, and it may not necessarily be in path, it could be their own python env

@jjw24
Copy link
Member Author

jjw24 commented Jun 18, 2021

This doesn't prompt user at all, just silently assigns if paths has python. I think we remove it so it prompts user to download portable or chooses to set it themselves. Essentially just two options

ok, got you. Maybe we can have another option to detect python path automatically?

Well my perspective, no need to do this, because python installation could be anywhere, no point in guessing where it could be.

@Zeroto521
Copy link
Member

I need some time to do this, I would keep watching this.

@jjw24
Copy link
Member Author

jjw24 commented Jun 18, 2021

I can get it ready, then we can test it out, if it's more intuitive to use the old code we can always add it back in

@jjw24 jjw24 marked this pull request as ready for review June 18, 2021 13:01
@jjw24 jjw24 linked an issue Jun 18, 2021 that may be closed by this pull request
@jjw24 jjw24 enabled auto-merge June 19, 2021 01:19
@jjw24
Copy link
Member Author

jjw24 commented Jun 19, 2021

@pc223
Copy link
Contributor

pc223 commented Jun 19, 2021

https://ci.appveyor.com/api/buildjobs/50ksa6gaei14mkff/artifacts/Output%2FPackages%2FFlow-Launcher-v1.7.2.exe

I just quick test this build, working pretty well, I tested with 2 plugins, Flow.Launcher.Plugin.Currency, Flow.Launcher.Plugin.Timestamp (Windows 10 21H1)

Python is downloaded silently, without a progress bar seems a bit awkward in terms of UX, but no big deal 😄

Python folder is Python Embeddable, personally, I try to avoid spaces in paths as much as possible, this might causing bugs in the future if devs don't aware of it. Maybe drop the space?

Copy link
Member

@deefrawley deefrawley left a comment

Choose a reason for hiding this comment

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

Tested, works with my CC plugin

@jjw24 jjw24 merged commit fd96d92 into dev Jun 19, 2021
@jjw24 jjw24 deleted the switch_python_embeddable branch June 19, 2021 04:04
@jjw24
Copy link
Member Author

jjw24 commented Jun 19, 2021

Python is downloaded silently, without a progress bar seems a bit awkward in terms of UX, but no big deal 😄

yeah true, we can add it later, we dont have a progress bar for anything except for the query window.

Python folder is Python Embeddable, personally, I try to avoid spaces in paths as much as possible, this might causing bugs in the future if devs don't aware of it. Maybe drop the space?

yeah makes sense, will do this in another pr

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Automatically install python dependency

6 participants