Skip to content

feat(build): Prune clones directory #6471

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

Open
Aradhya-Tripathi wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

feat(build): Prune clones directory #6471

Show file tree
Hide file tree
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: 8 additions & 0 deletions press/playbooks/prune_clones_directory.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
- name: Prune Clones Directory
hosts: all
become: yes
become_user: root
gather_facts: yes

@greptile-apps greptile-apps Bot May 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 gather_facts: yes collects host facts (OS, network, hardware) that are unused by this role's two tasks (a rm -rf and a file permission reset). The sibling docker_system_prune.yml uses gather_facts: no for the same reason — it adds unnecessary overhead per invocation.

Suggested change
gather_facts: yes
gather_facts: no
Prompt To Fix With AI 
This is a comment left during a code review.
Path: press/playbooks/prune_clones_directory.yml
Line: 6

Comment:
`gather_facts: yes` collects host facts (OS, network, hardware) that are unused by this role's two tasks (a `rm -rf` and a `file` permission reset). The sibling `docker_system_prune.yml` uses `gather_facts: no` for the same reason — it adds unnecessary overhead per invocation.

```suggestion
  gather_facts: no
```

How can I resolve this? If you propose a fix, please make it concise.

roles:
- role: prune_clones_directory
11 changes: 11 additions & 0 deletions press/playbooks/roles/prune_clones_directory/tasks/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
- name: Prune Clones Directory
command: rm -rf /home/frappe/agent/.clones/*

@greptile-apps greptile-apps Bot May 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 rm -rf with shell glob runs as root

The command module runs rm -rf /home/frappe/agent/.clones/* as root (via become_user: root). The files inside .clones are owned by frappe, so root is not strictly required for deletion — only the permission-reset step needs it. Consider dropping privileges for this task by adding become: no or become_user: frappe so the wildcard expansion and deletion run as the file owner rather than root.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: press/playbooks/roles/prune_clones_directory/tasks/main.yml
Line: 3

Comment:
**`rm -rf` with shell glob runs as root**

The `command` module runs `rm -rf /home/frappe/agent/.clones/*` as root (via `become_user: root`). The files inside `.clones` are owned by `frappe`, so root is not strictly required for deletion — only the permission-reset step needs it. Consider dropping privileges for this task by adding `become: no` or `become_user: frappe` so the wildcard expansion and deletion run as the file owner rather than root.

How can I resolve this? If you propose a fix, please make it concise.


- name: Set permissions back to frappe:frappe recursively
file:
path: /home/frappe/agent/.clones
state: directory
owner: frappe
group: frappe
recurse: yes
28 changes: 28 additions & 0 deletions press/press/doctype/server/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2412,6 +2412,15 @@ def prune_docker_system(self):
timeout=8000,
)

def prune_clone_directory(self):
frappe.enqueue_doc(
self.doctype,
self.name,
"_prune_clone_directory",
queue="long",
timeout=8000,
)

def _prune_docker_system(self, throw_on_failure: bool = False):
try:
ansible = Ansible(
Expand All @@ -2430,6 +2439,25 @@ def _prune_docker_system(self, throw_on_failure: bool = False):
frappe.throw("Failed to prune docker system") # nosemgrep
return None

def _prune_clone_directory(self, throw_on_failure: bool = False):
"""Prune clone directory to free up space on build server"""
try:
ansible = Ansible(
playbook="prune_clones_directory.yml",
server=self,
user=self._ssh_user(),
port=self._ssh_port(),
)
play = ansible.run()
if play.status != "Success" and throw_on_failure:
frappe.throw("Failed to prune clones directory") # nosemgrep
return play
except Exception:
log_error("Prune Build Directory Exception", doc=self)
if throw_on_failure:
Comment on lines +2455 to +2457

@greptile-apps greptile-apps Bot May 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 The exception log message says "Prune Build Directory Exception" instead of "Prune Clone Directory Exception". When this exception fires in production, searching logs for it will be misleading and point to the wrong operation.

Suggested change
except Exception:
log_error("Prune Build Directory Exception", doc=self)
if throw_on_failure:
except Exception:
log_error("Prune Clone Directory Exception", doc=self)
if throw_on_failure:
Prompt To Fix With AI 
This is a comment left during a code review.
Path: press/press/doctype/server/server.py
Line: 2455-2457

Comment:
The exception log message says `"Prune Build Directory Exception"` instead of `"Prune Clone Directory Exception"`. When this exception fires in production, searching logs for it will be misleading and point to the wrong operation.

```suggestion
		except Exception:
			log_error("Prune Clone Directory Exception", doc=self)
			if throw_on_failure:
```

How can I resolve this? If you propose a fix, please make it concise.

frappe.throw("Failed to prune clones directory") # nosemgrep
return None

def get_nat_gateway_ip(self):
if hasattr(self, "nat_server") and self.nat_server:
nat_private_ips = frappe.db.get_value(
Expand Down
Loading