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

added filter argument to tar.extactall when extracting downloaded file in container.download() #56

Merged
merged 1 commit into from
Apr 1, 2024

Conversation

JOUNAIDSoufiane
Copy link
Contributor

tar.extractall() or tar.extract() are now both deprecated if a filter argument is not specified.

see https://docs.python.org/3/library/tarfile.html#extraction-filters and mindsdb/mindsdb#8927

Copy link
Contributor

@Mark-Powers Mark-Powers left a comment

Choose a reason for hiding this comment

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

Looks good!

@@ -98,7 +98,7 @@ def create_container(
kwargs["exposed_ports"] = exposed_ports

# Note: most documented args are not on the function signature because there is some special
# handling of it in the Zun client; it is not sent if it is not on kwargs.
# handling of it in the Zu n client; it is not sent if it is not on kwargs.
Copy link
Contributor

Choose a reason for hiding this comment

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

typo?

@@ -236,7 +236,7 @@ def download(container_ref: "str", source: "str", dest: "str"):
res = zun().containers.get_archive(container_ref, source)
fd = io.BytesIO(res["data"])
with tarfile.open(fileobj=fd, mode="r") as tar:
tar.extractall(dest)
tar.extractall(dest, filter="fully_trusted")
Copy link
Contributor

Choose a reason for hiding this comment

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

does this cause any issues if you run an older version of the library?

@JOUNAIDSoufiane JOUNAIDSoufiane merged commit a71ba43 into master Apr 1, 2024
1 check passed
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