Skip to content
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

Python on path #258

Merged
merged 9 commits into from
Oct 17, 2023
Merged

Python on path #258

merged 9 commits into from
Oct 17, 2023

Conversation

mmarchetti
Copy link
Contributor

This PR changes how we handle finding the active Python executable, if one isn't specified.

We previously tried to find python on PATH; this PR adds a fallback to python since some installs do not include an executable named python3.

Intent

Fixes #172

Type of Change

  • Bug Fix
  • New Feature
  • Breaking Change

Automated Tests

There are unit tests covering this change.

Directions for Reviewers

Re-test the Windows scenario in #172 and verify that if python is on PATH but python3 is not, we use the python executable.

Copy link
Collaborator

@sagerb sagerb left a comment

Choose a reason for hiding this comment

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

Is it possible that you need to change the base of this PR, as it seems to include a lot more than just support for finding python or python3? Otherwise, if this is what you intended, you should update the description and get validation instructions in there for the other changes.

.vscode/launch.json Outdated Show resolved Hide resolved
cmd/connect-client/commands/publish.go Outdated Show resolved Hide resolved
@mmarchetti mmarchetti changed the base branch from main to mm-default-account October 5, 2023 12:36
Base automatically changed from mm-default-account to main October 10, 2023 19:58
@kgartland-rstudio
Copy link
Collaborator

kgartland-rstudio commented Oct 16, 2023

I suspect this isn't quite ready but I just grabbed the binary from GH Actions and it's still failing when I don't have a python3.exe. Here's a gif of what's I'm seeing when using this build:

windows-python

The error when there is no python3.exe:

time=2023-10-16T13:05:47.759-04:00 level=ERROR msg="Error running command" error="exit status 9009"

PYTHONPATH=C:\Users\kgartland\AppData\Local\Programs\Python\Python311
PATH=C:\Users\kgartland\AppData\Local\Programs\Python\Python311\

@kgartland-rstudio
Copy link
Collaborator

Looks like the culprit is the App Execution Alias setting, when disabling for python3.exe, the publish ui works:

Screenshot 2023-10-16 at 2 01 37 PM

By default that's enabled though, so we should find the appropriate app alias regardless of whether or not there is a python3.exe file.

@kgartland-rstudio
Copy link
Collaborator

Verifed fix! New build works with no python3.exe with and without App Execution Alias set.

@mmarchetti mmarchetti merged commit 20af44f into main Oct 17, 2023
14 checks passed
@mmarchetti mmarchetti deleted the mm-python-on-path branch October 17, 2023 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

publish-ui only scans for python3 on PATH
4 participants