Skip to content

⚡ Bolt: Parallelize folder deletion #146

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

Closed
abhimehro wants to merge 1 commit into from
Closed

⚡ Bolt: Parallelize folder deletion #146

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
31 changes: 24 additions & 7 deletions main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@
)
return False
if "group" not in data["group"]:
log.error(

Check warning

Code scanning / Prospector (reported by Codacy)

Use lazy % formatting in logging functions (logging-fstring-interpolation) Warning

Use lazy % formatting in logging functions (logging-fstring-interpolation)
f"Invalid data from {sanitize_for_log(url)}: Missing 'group.group' (folder name)."
)
return False
Expand Down Expand Up @@ -651,7 +651,7 @@
return [rule["PK"] for rule in folder_rules if rule.get("PK")]
except httpx.HTTPError:
return []
except Exception as e:

Check warning

Code scanning / Pylint (reported by Codacy)

Variable name "e" doesn't conform to snake_case naming style Warning

Variable name "e" doesn't conform to snake_case naming style
# We log error but don't stop the whole process;
# individual folder failure shouldn't crash the sync
log.warning(f"Error fetching rules for folder {folder_id}: {e}")
Expand Down Expand Up @@ -808,7 +808,7 @@
grp["PK"],
)
return str(grp["PK"])
except Exception as e:

Check notice

Code scanning / Pylintpython3 (reported by Codacy)

Catching too general exception Exception Note

Catching too general exception Exception
log.debug(f"Could not extract ID from POST response: {e}")

# 2. Fallback: Poll for the new folder (The Robust Retry Logic)
Expand All @@ -835,7 +835,7 @@
)
time.sleep(wait_time)

log.error(

Check notice

Code scanning / Pylintpython3 (reported by Codacy)

Use lazy % formatting in logging functions Note

Use lazy % formatting in logging functions
f"Folder {sanitize_for_log(name)} was not found after creation and retries."
)
return None
Expand Down Expand Up @@ -1149,16 +1149,33 @@
existing_folders = list_existing_folders(client, profile_id)
if not no_delete:
deletion_occurred = False
# Use a dict to deduplicate folders to delete
folders_to_delete = {}
for folder_data in folder_data_list:
name = folder_data["group"]["group"].strip()
if name in existing_folders:
# Optimization: Maintain local state of folders to avoid re-fetching
# delete_folder returns True on success
if delete_folder(
client, profile_id, name, existing_folders[name]
):
del existing_folders[name]
deletion_occurred = True
folders_to_delete[name] = existing_folders[name]

if folders_to_delete:
# Parallelize deletion: 5 workers is generally safe for DELETE operations
with concurrent.futures.ThreadPoolExecutor(max_workers=5) as executor:
future_to_folder = {
executor.submit(
delete_folder, client, profile_id, name, folder_id
): name
for name, folder_id in folders_to_delete.items()
}

for future in concurrent.futures.as_completed(future_to_folder):
name = future_to_folder[future]
try:
if future.result():
del existing_folders[name]
deletion_occurred = True
except Exception as e:

Check warning

Code scanning / Pylintpython3 (reported by Codacy)

Variable name "e" doesn't conform to snake_case naming style Warning

Variable name "e" doesn't conform to snake_case naming style
log.error(
f"Failed to delete folder {sanitize_for_log(name)}: {e}"
)
Comment on lines +1176 to +1178
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Jan 30, 2026

Choose a reason for hiding this comment

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

security-medium medium

The newly added parallel folder deletion logic catches exceptions but fails to sanitize the exception object e before logging it. This could allow log injection or terminal manipulation if the exception message contains malicious content. The project already uses sanitize_for_log elsewhere, making this a security regression. Additionally, consider renaming the name variable in the dictionary comprehension (around line 1166) to avoid shadowing and improve readability, as suggested by the code review.

Suggested change
log.error(
f"Failed to delete folder {sanitize_for_log(name)}: {e}"
)
log.error(
f"Failed to delete folder {sanitize_for_log(name)}: {sanitize_for_log(e)}"
)

Comment on lines +1159 to +1178
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

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

While the implementation follows existing patterns and the PR mentions verification through benchmarking, adding a test for the parallel deletion functionality would be beneficial for long-term maintainability. The codebase includes tests for other concurrent operations (see tests/test_cache_optimization.py lines 82-157 for examples of testing thread-safe concurrent operations). Consider adding a similar test that verifies parallel deletion works correctly and handles failures appropriately.

Copilot uses AI. Check for mistakes.

# CRITICAL FIX: Increased wait time for massive folders to clear
if deletion_occurred:
Expand All @@ -1176,7 +1193,7 @@
with concurrent.futures.ThreadPoolExecutor(
max_workers=max_workers
) as executor:
future_to_folder = {

Check notice

Code scanning / Pylint (reported by Codacy)

Catching too general exception Exception Note

Catching too general exception Exception
executor.submit(
_process_single_folder,
folder_data,
Expand Down
Loading