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

update the project archive sync support for py2 compatability #8057

Merged

Conversation

ryanpetrello
Copy link
Contributor

@ryanpetrello ryanpetrello commented Sep 1, 2020

playbook runs in AWX are still supported in python2, so this adds
support for archive-based project syncs in py2

cc @philipsd6

playbook runs in AWX are still supported in python2, so this adds
support for archive-based project syncs in py2
except BadZipFile:
try:
archive = tarfile.open(src)
except tarfile.ReadError:
Copy link
Contributor Author

@ryanpetrello ryanpetrello Sep 1, 2020

Choose a reason for hiding this comment

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

This lack of indentation was actually a legitimate bug we just missed in code review.

try:
from zipfile import BadZipFile
except ImportError:
from zipfile import BadZipfile as BadZipFile # py2 compat
Copy link
Contributor Author

@ryanpetrello ryanpetrello Sep 1, 2020

Choose a reason for hiding this comment

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

Booooooooooooo python2

try:
is_dir = member.isdir()
except AttributeError:
is_dir = member.filename[-1] == '/' # py2 compat for ZipInfo
Copy link
Contributor Author

Choose a reason for hiding this comment

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

python2's zipfile doesn't have this, but this is what py3 does, so I just copied it here


if is_dir:
os.makedirs(dest, exist_ok=True)
try:
os.makedirs(dest)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

poor man's exist_ok=True (which also doesn't exist in py2)

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

Copy link
Member

@kdelee kdelee left a comment

Choose a reason for hiding this comment

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

👍 fixes the tests that were failing, works for me!

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit b11a5c1 into ansible:devel Sep 1, 2020
@philipsd6
Copy link
Contributor

@ryanpetrello I had the understanding that AWX was solidly Python3 at this point, and even if a project was associated with a Py2 venv, the project sync would be done with the default Py3, no? Seems unreasonable for the built ins to not be able to depend on the main Python version! In any case, as my understanding was apparently wrong, thanks for catching this and fixing it!

P.S. I'm so tired of writing backwards compatible Python... Sometimes I even get bit when using {k: v for k, v in ...} instead of dict((k, v) for k, v in ...)

@ryanpetrello
Copy link
Contributor Author

ryanpetrello commented Sep 2, 2020

Hey @philipsd6,

Hey no big deal. I'm a maintainer and I forgot too 😂.

When a project update runs, it uses the default "Ansible" virtualenv, and we still technically support systems (like RHEL7) that have python2 as their default system Python for playbook execution.

The Python code that AWX itself runs in its various processes is all python3 only, though.

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.

3 participants