Skip to content

Fix python feature issue #258, improve tests, fix oryx bug - #261

Merged
Chuck Lantz (Chuxel) merged 4 commits into
mainfrom
fix-258
Nov 7, 2022
Merged

Fix python feature issue #258, improve tests, fix oryx bug#261
Chuck Lantz (Chuxel) merged 4 commits into
mainfrom
fix-258

Conversation

@Chuxel

Copy link
Copy Markdown
Member

As described in #258, currently the "defaultInterpreterPath" is on correct in a number of cases since the code and path in the python feature was modified to support more than one version of python.

I fixed the additional python test that would have returned false positives (the | grep ... was for the test command so it always was successful).

Since the feature now manages a set of symlinks, I added test coverage for those since that was also part of the problem with os-provided... no symlink was created.

I then added one for oryx. That exposed a bug where the target folder may be missing so I fixed that.

@Chuxel
Chuck Lantz (Chuxel) requested a review from a team as a code owner November 5, 2022 04:23
@Chuxel

Copy link
Copy Markdown
Member Author

/cc John Keech (@jkeech) who replied to #258

@Chuxel

Copy link
Copy Markdown
Member Author

The Debian 9 test failed since stretch is completely out of support now and Python 3.6 is as well - so none of the tools will install in it. I'd suggest removing the test and making clear we don't support it in any feature at this point.

Happy to do that in this PR if it makes sense.

joshspicer
joshspicer previously approved these changes Nov 7, 2022

@joshspicer joshspicer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No comments - this looks great to me, thanks!

@joshspicer

Copy link
Copy Markdown
Member

Happy to do that in this PR if it makes sense.

That would be great, go for it please :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks great! Thanks for fixing this and adding test coverage.

Happy to do that in this PR if it makes sense.

+1 Makes sense!

Comment thread src/python/install.sh Outdated
Comment thread src/python/install.sh
@Chuxel
Chuck Lantz (Chuxel) merged commit 3020d6d into main Nov 7, 2022
@Chuxel
Chuck Lantz (Chuxel) deleted the fix-258 branch November 7, 2022 18:46
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.

4 participants