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.

Sign up for GitHub

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

Allow to recursively search package #1270

Closed
wants to merge 1 commit into from
Closed

Allow to recursively search package #1270

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions zappa/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,9 @@ def splitpath(path):
else:
copytree(cwd, temp_project_path, metadata=False, symlinks=False)
for glob_path in exclude_glob:
for path in glob.glob(os.path.join(temp_project_path, glob_path)):
# Use `recursive` to match paths deep in the directory tree
# https://github.com/zappa/Zappa/issues/1269
for path in glob.glob(os.path.join(temp_project_path, glob_path), recursive=True):
Copy link
Member

Choose a reason for hiding this comment

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

Do you think there's any downside to using glob.iglob() here instead of glob.glob(), as iglob() returns an iterator, which should be easier on memory usage if a user decides to put massive folders with tons of files in exclude_glob? Especially since this code may often running be running on a CI runner, and the free ones that Github/Gitlab provide aren't provisioned with very much RAM at all (IIRC, it's like 1GB, and most is taken up my the OS and CI runner software).

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, yes, that's a great idea.

try:
os.remove(path)
except OSError: # is a directory
Expand Down Expand Up @@ -767,7 +769,9 @@ def splitpath(path):

# Cleanup
for glob_path in exclude_glob:
for path in glob.glob(os.path.join(temp_project_path, glob_path)):
# Use `recursive` to match paths deep in the directory tree
# https://github.com/zappa/Zappa/issues/1269
for path in glob.glob(os.path.join(temp_project_path, glob_path), recursive=True):
Copy link
Member

Choose a reason for hiding this comment

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

Same question of using glob.iglob() instead of glob.glob(). (See other comment)

try:
os.remove(path)
except OSError: # is a directory
Expand Down