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

Use python3 from $PATH instead of absolute paths #1466

Open
wants to merge 1 commit into
base: fb-mysql-8.0.32
Choose a base branch
from

Conversation

laurynas-biveinis
Copy link
Contributor

This allows using Python virtualenv directly. This partially reverts changes in
9d0dda0, but its commit message is not stating
anything about absolute paths.

@facebook-github-bot
Copy link

@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@luqun
Copy link
Contributor

luqun commented Jul 4, 2024

after change from /usr/bin/python3 to pythohn3, some of MTR failed to find due to "ModuleNotFoundError: No module named 'MySQLdb'".

@laurynas-biveinis
Copy link
Contributor Author

Do you know what python3 are the scripts picking up? Is it possible to adjust PATH to put /usr/bin before that?

@luqun
Copy link
Contributor

luqun commented Jul 5, 2024

Do you know what python3 are the scripts picking up? Is it possible to adjust PATH to put /usr/bin before that?

the pyhon3 in /usr/local/sbin instead of /usr/bin/python3.. need to do some investigation since this PATH variable comes from SYSTEM.

@laurynas-biveinis
Copy link
Contributor Author

laurynas-biveinis commented Jul 8, 2024 via email

@laurynas-biveinis
Copy link
Contributor Author

As discussed offline, will conditionalize on the OS

@laurynas-biveinis laurynas-biveinis marked this pull request as draft August 5, 2024 16:31
@laurynas-biveinis
Copy link
Contributor Author

@luqun , @hermanlee , could you tell which tests break with this PR in its current form? I am wondering if I could leave script shebangs using env

@luqun
Copy link
Contributor

luqun commented Aug 29, 2024

@luqun , @hermanlee , could you tell which tests break with this PR in its current form? I am wondering if I could leave script shebangs using env

such as main.admission_control_multi_query_wait_events

@facebook-github-bot
Copy link

@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

This allows using Python virtualenv directly. This partially reverts changes in
9d0dda0, but its commit message is not stating
anything about absolute paths. For some tests, do this conditionally based on
the platform due to some test hosts requiring the hardcoded paths.
@facebook-github-bot
Copy link

@laurynas-biveinis has updated the pull request. You must reimport the pull request before landing.

@laurynas-biveinis
Copy link
Contributor Author

@luqun , @hermanlee , could you tell which tests break with this PR in its current form? I am wondering if I could leave script shebangs using env

such as main.admission_control_multi_query_wait_events

I have updated the three tests in the main suite to choose $PATH or hardcoded python conditionally. Let me know if any other tests need updating

@laurynas-biveinis laurynas-biveinis marked this pull request as ready for review August 30, 2024 08:56
@facebook-github-bot
Copy link

@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

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

Successfully merging this pull request may close these issues.

3 participants