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-3.12 compat and refactor of path determination. #845

Merged

Conversation

abadger
Copy link
Member

@abadger abadger commented Oct 17, 2023

In stdlib._call(), we lookup whether the command that we are going to call is present on the user's PATH. Two changes are made to the function:

  • Simplify the lookup of PATH now that we are making a copy of the environment's PATH.
  • Add compat code to use shutil.which() on Python3 (while still using distutils.spawn.find_executable() on Python2). As noted in the comment, which() will be needed when the code runs on Python-3.12.

In `stdlib._call()`, we lookup whether the command to call is present
on the user's PATH.  Two changes are made to this functionality:

* Simplify the lookup of PATH now that we have a local variable with all the
  information we need.
* Add compat code to use `shutil.which()` on Python3 (while still using
  `distutils.spawn.find_executable()` on Python2).  As noted in pstodulk's comment, `which()`
  will be needed when the code runs on Python-3.12.
@github-actions
Copy link

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergeable.
If you want to request a review or rebuild a package in copr, you can use following commands as a comment:

  • review please @oamg/developers to notify leapp developers of the review request
  • /packit copr-build to submit a public copr build using packit

To launch regression testing public members of oamg organization can leave the following comment:

  • /rerun to schedule basic regression tests using this pr build and leapp-repository*master* as artifacts
  • /rerun 42 to schedule basic regression tests using this pr build and leapp-repository*PR42* as artifacts
  • /rerun-sst to schedule sst tests using this pr build and leapp-repository*master* as artifacts
  • /rerun-sst 42 to schedule sst tests using this pr build and leapp-repository*PR42* as artifacts

Please open ticket in case you experience technical problem with the CI. (RH internal only)

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please consider rerunning the CI by commenting leapp-ci build (might require several comments). If the problem persists, contact leapp-infra.

Copy link
Member

@pirat89 pirat89 left a comment

Choose a reason for hiding this comment

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

it makes completely sense. no idea why it didn't come to my mind originally. thanks :) just one liner change is needed

leapp/libraries/stdlib/call.py Show resolved Hide resolved
@pirat89 pirat89 merged commit 034aee0 into oamg:master Oct 24, 2023
15 checks passed
@pirat89 pirat89 added the changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant label Oct 24, 2023
@abadger abadger deleted the leapp-refactor-path-determination-in-call branch October 25, 2023 09:50
pirat89 added a commit to pirat89/leapp that referenced this pull request Feb 13, 2024
## Framework
### Enhancements
- Generated txt report file now contains also External links when part of a report (oamg#842, oamg#844)

## stdlib
### Fixes
- The `run` function is now compatible with Python 3.12 (oamg#845)
@pirat89 pirat89 mentioned this pull request Feb 13, 2024
pirat89 added a commit that referenced this pull request Feb 13, 2024
## Framework
### Enhancements
- Generated txt report file now contains also External links when part of a report (#842, #844)

## stdlib
### Fixes
- The `run` function is now compatible with Python 3.12 (#845)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants