-
Notifications
You must be signed in to change notification settings - Fork 339
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
Issue #1930 Replaced for code based on ssl.get_server_certificate(). #1967
Issue #1930 Replaced for code based on ssl.get_server_certificate(). #1967
Conversation
…ver_certificate().
Build failed. ✔️ ansible-tox-linters SUCCESS in 9m 43s |
recheck |
I don't think that rechecking will help. The CI fails in
This doesn't look related to your changes because I think this module doesn't use FYI this looks suspiciously like something mentioned in the forum. I'll have to investigate this a bit... |
I saw that the problem is in vmware_content_library_info. But anyway, first I want to get a second result of the same kind before conducting further investigation. Knowing our CI >_< |
I've created #1968 to investigate this further. Maybe we can fix the issue like this: community.vmware/plugins/connection/vmware_tools.py Lines 304 to 306 in 9718381
I'll work on it and let you know if I found a solution. |
Build succeeded. ✔️ ansible-tox-linters SUCCESS in 9m 35s |
What is this?! I thought I would understand why the CI failed, but now I don't understand why it started to succeed again... I realyy hate things like this... |
@mariolenz As I said earlier - knowing our CI, until I get a 100% reproducible problem, I will not investigate. 😂 |
Build failed. ✔️ ansible-tox-linters SUCCESS in 9m 42s |
recheck |
Build succeeded. ✔️ ansible-tox-linters SUCCESS in 9m 42s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
But it would be great if @Fredouye would give us some feedback on this.
I can't test this at the moment with Python 3.12. I was hoping @Fredouye would test this but we didn't get any feedback :-/ As an alternative, I thought we could make the CI test your code with Python 3.12. So I've opened ansible/ansible-zuul-jobs#1844 and #1974 but it looks like there is a problem with Python 3.12. I'll try to make the CI succeed with Python 3.12 and then we could test you changes there. However, I'd like to do a new release soon and would like to have your changes there. Should we merge in 2 or 3 days, anyway? I mean, it doesn't look like your changes break anything (CI succeeded) even if we're not 100% sure that your PR really fixes the issues. |
It looks like the vSphere Automation Python SDK has some problems with Python 3.12 (vmware/vsphere-automation-sdk-python#407 and vmware/vsphere-automation-sdk-python#400). So I don't see how our CI could succeed until they've fixed this. I'm not 100% sure if this PR fixes #1930 because I can't test at the moment, but it looks like it does to me. So let's merge, especially since the CI is happy so it shouldn't break anything. Thanks @ihumster! |
Build succeeded (gate pipeline). ✔️ ansible-tox-linters SUCCESS in 9m 28s |
754e6c1
into
ansible-collections:main
SUMMARY
Replaced for code based on ssl.get_server_certificate().
Fixes #1930
ISSUE TYPE