ansible-collections / community.kubernetes

Kubernetes Collection for Ansible
https://galaxy.ansible.com/community/kubernetes
GNU General Public License v3.0
265 stars 106 forks source link

test: older_openshift_fail.yml fails with py3.7+ #291

Closed goneri closed 3 years ago

goneri commented 3 years ago

The older_openshift_fail.yml test playbook fails with Python 3.7 because the old kubernetes.client.apis.admissionregistration_api module uses the new async keyword as a function parameter name.

From: https://docs.python.org/3/whatsnew/3.7.html

Backwards incompatible syntax changes:

`async` and `await` are now reserved keywords.

In this case, we get a SyntaxError exception instead of the expected Failed to import the required Python library (openshift >= 0.7.2).

codecov[bot] commented 3 years ago

Codecov Report

Merging #291 into main will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #291   +/-   ##
=======================================
  Coverage   37.00%   37.00%           
=======================================
  Files           3        3           
  Lines         727      727           
  Branches      144      144           
=======================================
  Hits          269      269           
  Misses        409      409           
  Partials       49       49           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 77e48d0...0695b2a. Read the comment docs.

goneri commented 3 years ago

Can we get this one merged?

tima commented 3 years ago

@geerlingguy and I were discussing this and think the test can just be nuked. It served its purpose, but now it's checking for something rather old.

geerlingguy commented 3 years ago

Yeah—it was 2018 when the last major release of 0.6.x came out. We're still testing for a bug (that I remember from then) that's been resolved for over two years of new releases...

I believe either @willthames or @fabianvf originally wrote that test though, and might have more missing context—it might be nice to get one final "yes" from one of them before totally nuking it though.

tima commented 3 years ago

I'm going to merge this for now, but as mentioned earlier and in #300 we'd like to remove this test entirely.