-
Notifications
You must be signed in to change notification settings - Fork 199
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
Adds support for special ansible_python_interpreter values, ansible_python_interpreter discovery, and fixes tests #658
Adds support for special ansible_python_interpreter values, ansible_python_interpreter discovery, and fixes tests #658
Conversation
If I'm going about this the wrong way please let me know; I ran into an issue where this was broken (but not when using vanilla ansible): |
… so python connects
I'm gonna run more tests on my end and then add a test for the new functionality + docs.
|
Fails on a more complex command:
I'll try and fix that up as well |
PR ready! Passed all automation on my side, and passes |
…rpreter functionality
@dw this PR is ready for review now 😄 |
I'll rerun tests; commit adding a few sentences to a readme file is unrelated to ^^^ |
I haven't used azure devops before, not sure how to kick off a rebuild. There's no rebuild button that I can see from https://dev.azure.com/dw-mitogen/Mitogen/_build/results?buildId=696&view=ms.vss-test-web.build-test-results-tab . The docs say that rebuild is available in the latest version of |
Any chance you can provide your patch without the formatting cleanups to code you didn't touch. Its actually fairly hard to see your changes. Also, is this supposed to support the things like ansible_python_interpreter=auto special flags like are documented here: |
This, alongside ansible 2.9 are two critical features that prevent me from attempting to test it as I know I need both. |
For sure! Sorry for the late reply, I recently came back from vacation and have been really busy since so haven't had time to work on this yet. The formatting changes are due to my editor auto-linting files I save if I made any changes to that file in general. I will disable my auto-linter and revert the pep8 formatting changes though. Lastly, I don't think this patch will fix special interpreter values of things like I will be able to work on changes to this PR soon! |
…act isn't being cached
able to detect interpreters now! My test cases taken mostly from https://github.com/ansible/ansible/pull/50163/files#diff-2efff5ba3589af60c1010f00a234ef57 still don't fully pass yet though |
Now the other test failed D: these tests are too flaky... |
travis is under maintenance now. Once it's back up, if tests are solid I'll wait a day or two for more people to try the patch, but I really want to get it in soon so I can start getting ansible 2.9 support working (this PR also fixes tests which is why it should come in first) |
Hello, I currently have this same issue using your latest commit. I'm using Ansible 2.9.6 Here's a line of the -vvvv showing it's still trying to use /usr/bin/python
Here's my ansible.cfg (as you can see I also tried setting to auto with the same results)
Here's my pertinent environment variables
On the target machine (the machine I'm running Ansible against), there is only a |
Can you try with Ansible 2.8.8? Ansible 2.9.6 support isn't fully available yet (some playbooks work and some don't); it might still be broken but it would be good to rule that out first |
@s1113950 - 2.8.8 has always worked wonderfully. That's what I've been using before posting this. I didn't realize this wasn't fully supported in 2.9.6. What's the best way to keep my finger on the pulse of what's supported when regarding 2.9.6? |
@cbrum11 it should be supported in 2.9.6, just wanted to check if there's a case where it doesn't because I originally added the feature using 2.8.8. However, I recently updated the tests to test 2.9.6 as well though and they passed interpreter discovery 🤔 Does interpreter discovery work without mitogen for you on Ansible 2.9.6? Mitogen calls Ansible's If interpreter discovery works for you without Mitogen, I'll need your host and target OS in order to better debug your issue :) As of right now, afaik everything should be supported in Ansible 2.9.6 as of #705 except for collections. There's a big discussion on collections (and Mitogen backwards compat/new releases) here: #715 . |
Thanks for the super speedy response. I believe interpreter discovery does work on 2.9.6 without mitogen, but I'll give it a specific test to make 100% sure this week. |
I have just stumbled into this problem. The server is an ubuntu-netinst VM without anything on it. It has only /usr/bin/python3, which Is may not be relevant unless this MR is already published there. |
This PR will be in the next release; 0.2.9 doesn't have it. You can pip install from master of this repo to get the fix for now :) a new release(s) is planned for after Ansible 2.10 comes out |
Hi, what would the command to install from master be in this case? I am, unfortunately, still not too familiar with pip. Thank you! |
@fourstepper |
For |
Ok it should be It seems it is working now and it is faster. Thank you very much for fixing it so that Mitrogen can save my time. Reduced from 2:01 minutes to 1:21, amazing |
I am a little late to the party, but was the issue resolved in regards to the unhandled python interpreter warning message? I am running ansible 2.8.12 via mitogen and receiving that warning:
Wondering what configuration updates need to be done on my end if any. Thanks in advance for any help. |
@lancefriday945 either of these tags should have the fix in them: |
I got that cleaned up. Were you working on a fix for the unreachable issue?
I am still experiencing that issue. Do I need to put my config somewhere else for ssh retry logic? Right now I have this set in my ansible.cfg file:
I also have group_vars folders for our different environments. |
@lancefriday945 I'm not sure about the connection issue, I don't think it relates specifically to the python interpreter patch. Mitogen should use the same logic that Ansible does for setting ssh_connection retries |
Fixes #597 , fixes #680 , fixes #673
adds support for Jinja templating in
ansible_python_interpreter
, adds support foransible_python_interpreter
discovery, and fixes up some test issues I ran into in order to get this PR passing (tests hadn't passed on a PR since last year).Additionally I had to update the Mac VM to 10.14; MS said they'll stop supporting 10.13 soon: https://docs.microsoft.com/en-us/azure/devops/pipelines/agents/hosted?view=azure-devops#use-a-microsoft-hosted-agent and also updated away from ubuntu 16.04:
https://dev.azure.com/dw-mitogen/Mitogen/_build/results?buildId=793&view=logs&j=364fd14b-95c5-5834-280a-1629ce194374&t=2241d271-1077-5b70-19c7-218604a5902c
sometimes this happened... 🤦♂
I ported over the tests found here: https://github.com/ansible/ansible/pull/50163/files#diff-2efff5ba3589af60c1010f00a234ef57R1 from ansible where interpreter discovery was first added, and added them to the CI to verify interpreter discovery works.
Also, I leveraged the existing
discover_interpreter
function from Ansible, which means that interpreter discovery won't work in Ansible < 2.8.0 but it already doesn't work in those Ansible versions so I didn't think this was an issue.Finally, I disabled a bunch of tests that worked locally but wouldn't pass on CI. I made new github issues out of the fails and labeled them with
ci
.