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

Ansible galaxy role download should not have to perform aditionnal ch… #56617

Merged
merged 1 commit into from Jun 13, 2019

Conversation

Projects
None yet
4 participants
@arenard
Copy link
Contributor

commented May 19, 2019

…eck against tar archive file extension #56616.

SUMMARY

Ansible galaxy role download performs unnecessary check against file extension and specifically against .tar.gz.
Tarfile tarfile.is_tarfile(...) method seems sufficient to identify if the file is a valid tarfile and if tarfile is able to open it.
This allows to open file by using transparent stream compression of tarfile. This makes .tar.gz and .tar.bz2 formats valid with python 2.6.x/2.7.x (as well as .tar.xz with python 3.x).

ISSUE TYPE
  • Docs Pull Request
  • Feature Pull Request
COMPONENT NAME

ansible-galaxy

ADDITIONAL INFORMATION

Ansible galaxy role download should not have to perform aditionnal ch…
…eck against tar archive file extension #56616.

Ansible galaxy role download performs unnecessary check against file extension and specifically against `.tar.gz`.
Tarfile `tarfile.is_tarfile(...)` method seems sufficient to identify if the file is a valid tarfile and if tarfile is able to open it.
This allows to open file by using transparent stream compression of tarfile. This makes `.tar.gz` and `.tar.bz2` formats valid with python 2.6.x/2.7.x (as well as `.tar.xz` with python 3.x).
@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 19, 2019

@orthanc

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2019

I've had a look over the codechange after this got raised in #ansible-docs. Haven't tested yet but this looks like a good, safe change to me.

@orthanc

This comment has been minimized.

Copy link
Contributor

commented Jun 13, 2019

Looks Good To Me

I've tested out the code and it does what it says on the tin. I can't see any reason to keep the current limit to gzipped tars when the underlying library supports a wider range.

@acozine

This comment has been minimized.

Copy link
Contributor

commented Jun 13, 2019

Thanks @arenard for the PR and @orthanc for the review. It's good to make our tools flexible and adaptable.

@acozine acozine merged commit 1a5fd72 into ansible:devel Jun 13, 2019

1 check passed

Shippable Run 123509 status is SUCCESS.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.