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

feat: add direct source code to clean up empty folders instead of depending on additional plugin #48

Closed
wants to merge 10 commits into from
7 changes: 1 addition & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,6 @@ artifactory-cleanup --destroy --user user --password password --artifactory-serv
# debug run - only print founded artifacts. it do not delete
artifactory-cleanup --user user --password password --artifactory-server https://repo.example.com/artifactory --config reponame.py

# Clean up empty folder
# --remove-empty-folder
# You need to use the plugin https://github.com/jfrog/artifactory-user-plugins/tree/master/cleanup/deleteEmptyDirs to delete empty folders
artifactory-cleanup --remove-empty-folder --user user --password password --artifactory-server https://repo.example.com/artifactory

# Debug run only for ruletestname. Find any *ruletestname*
# debug run - only print founded artifacts. it do not delete
artifactory-cleanup --rule-name ruletestname --user user --password password --artifactory-server https://repo.example.com/artifactory --config reponame.py
Expand Down Expand Up @@ -157,4 +152,4 @@ docker build . --tag artifactory-cleanup
In order to provide a new release of `artifactory-cleanup`, there are two steps involved.

1. Bump the version in the [setup.py](setup.py)
2. Create a Git release tag (e.g. `v0.3.3`) by creating a release on Github
2. Create a Git release tag (e.g. `v0.3.3`) by creating a release on Github
27 changes: 7 additions & 20 deletions artifactory_cleanup/artifactorycleanup.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
from requests.auth import HTTPBasicAuth
from artifactory_cleanup.context_managers import get_context_managers
from artifactory_cleanup.rules.base import CleanupPolicy
from artifactory_cleanup.rules.delete import delete_empty_folder


requests.packages.urllib3.disable_warnings()
Expand Down Expand Up @@ -61,10 +60,6 @@ class ArtifactoryCleanup(cli.Application):
mandatory=False,
)

_remove_empty_folder = cli.Flag(
"--remove-empty-folder", help="Cleaning up empty folders in local repositories"
)

_days_in_future = cli.SwitchAttr(
"--days-in-future",
help="Simulate future behaviour",
Expand All @@ -83,21 +78,13 @@ def _destroy_or_verbose(self):
def main(self):
# remove trailing slash
self._artifactory_server = self._artifactory_server.rstrip("/")
if self._remove_empty_folder:
Pro marked this conversation as resolved.
Show resolved Hide resolved
rules = [
CleanupPolicy(
"Cleaning up empty folders in local repositories",
delete_empty_folder(),
)
]
else:
try:
self._config = self._config.replace(".py", "")
sys.path.append(".")
rules = getattr(importlib.import_module(self._config), "RULES")
except ImportError as error:
print("Error: {}".format(error))
exit(1)
try:
self._config = self._config.replace(".py", "")
sys.path.append(".")
rules = getattr(importlib.import_module(self._config), "RULES")
except ImportError as error:
print("Error: {}".format(error))
exit(1)

self._destroy_or_verbose()

Expand Down
111 changes: 80 additions & 31 deletions artifactory_cleanup/rules/delete.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from datetime import timedelta
from collections import defaultdict, deque

from artifactory_cleanup.rules.base import Rule

Expand Down Expand Up @@ -92,37 +93,85 @@ class delete_empty_folder(Rule):
"""

def _aql_add_filter(self, aql_query_list):
update_dict = {
"repo": {
"$match": "deleteEmptyFolder",
}
}
aql_query_list.append(update_dict)
# Get list of all files and folders
Pro marked this conversation as resolved.
Show resolved Hide resolved
all_files_dict = {"path": {"$match": "**"}, "type": {"$eq": "any"}}
aql_query_list.append(all_files_dict)
return aql_query_list

def _filter_result(self, result_artifact):
r = self.artifactory_session.get(
"{}/api/repositories?type=local".format(self.artifactory_server)
)
r.raise_for_status()
repositories = r.json()

for count, repository in enumerate(repositories, start=1):
if repository["packageType"] == "GitLfs":
# GitLfs should be handled by the jfrog cli: https://jfrog.com/blog/clean-up-your-git-lfs-repositories-with-jfrog-cli/
print(
f"Skipping '{repository['key']}' because it is a Git LFS repository"
)
continue

url = "{}/api/plugins/execute/deleteEmptyDirsPlugin?params=paths={}".format(
self.artifactory_server, repository["key"]
)

print(
f"Deleting empty folders for '{repository['key']}' - {count} of {len(repositories)}"
)
r = self.artifactory_session.post(url)
r.raise_for_status()

return []

# convert list of files to dict
# Source: https://stackoverflow.com/a/58917078

def nested_dict():
Copy link
Member

Choose a reason for hiding this comment

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

Inline functions are not readable here, it's hard to get the idea from it. Could we move as much as possible outside of the class?
I see there's no self usages here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will move this to a new utils.py file. It's more readable then.
I would still keep it as inline functions since this is a function very specific to the conversion of list to dict, and not used anywhere else.

Copy link
Member

Choose a reason for hiding this comment

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

It might be either a separate function or class method, but differently not a function inside class method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did some refactoring now. From my point of view it's now ready to be merged.

I would still argue against putting this as a separate function. It would be out of context. If you still disagree, feel free to direclty update this MR.

"""
Creates a default dictionary where each value is an other default dictionary.
"""
return defaultdict(nested_dict)

def default_to_regular(d):
"""
Converts defaultdicts of defaultdicts to dict of dicts.
"""
if isinstance(d, defaultdict):
d = {k: default_to_regular(v) for k, v in d.items()}
return d

def get_path_dict(artifacts):
Pro marked this conversation as resolved.
Show resolved Hide resolved
new_path_dict = nested_dict()
for artifact in artifacts:
parts = artifact["path"].split("/")
if parts:
marcher = new_path_dict
for key in parts:
# We need the repo for the root level folders. They are not in the
# artifacts list
marcher[key]["data"] = {"repo": artifact["repo"]}
marcher = marcher[key]["children"]
marcher[artifact["name"]]["data"] = artifact
return default_to_regular(new_path_dict)

artifact_tree = get_path_dict(result_artifact)

# Artifactory also returns the directory itself. We need to remove it from the list
# since that tree branch has no children assigned
if "." in artifact_tree:
del artifact_tree["."]

# Now we have a dict with all folders and files
# An empty folder is represented if it is a dict and does not have any keys

def get_folder_artifacts_with_no_children(item, path=""):

empty_folder_artifacts = deque()
Pro marked this conversation as resolved.
Show resolved Hide resolved
Pro marked this conversation as resolved.
Show resolved Hide resolved

def _add_to_del_list(key):
Pro marked this conversation as resolved.
Show resolved Hide resolved
empty_folder_artifacts.append(item[key]["data"])
# Also delete the item from the children list to recursively delete folders
# upwards
del item[key]

for x in list(item.keys()):
Pro marked this conversation as resolved.
Show resolved Hide resolved
if "type" in item[x]["data"] and item[x]["data"]["type"] == "file":
Pro marked this conversation as resolved.
Show resolved Hide resolved
continue
if not "path" in item[x]["data"]:
# Set the path and name for root folders which were not explicitly in the
# artifacts list
item[x]["data"]["path"] = path
item[x]["data"]["name"] = x
if not "children" in item[x] or len(item[x]["children"]) == 0:
# This an empty folder
_add_to_del_list(x)
else:
artifacts = get_folder_artifacts_with_no_children(
item[x]["children"], path=path + "/" + x if len(path) > 0 else x
)
if len(item[x]["children"]) == 0:
Pro marked this conversation as resolved.
Show resolved Hide resolved
# just delete the whole folder since all children are empty
_add_to_del_list(x)
else:
empty_folder_artifacts.extend(artifacts)

return empty_folder_artifacts

return list(get_folder_artifacts_with_no_children(artifact_tree))
2 changes: 1 addition & 1 deletion docs/RULES/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
| `delete_without_downloads()` | Deletes artifacts that have never been downloaded (DownloadCount=0). Better to use with `delete_older_than` rule |
| `delete_older_than_n_days_without_downloads(days=N)` | Deletes artifacts that are older than N days and have not been downloaded |
| `delete_not_used_since(days=N)` | Delete artifacts that were downloaded, but for a long time. N days passed. Or not downloaded at all from the moment of creation and it's been N days |
| `delete_empty_folder()` | Clean up empty folders in local repositories. A special rule that runs separately on all repositories. Refers to [deleteEmptyDirs](https://github.com/jfrog/artifactory-user-plugins/tree/master/cleanup/deleteEmptyDirs) plugin |
| `delete_empty_folder()` | Clean up empty folders in given repository list |
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a note about delete_empty_folder at the end

If you use Artifactory higher then 7.8.1 - it removes empty folders automaticly. We use the rule for some advanced usages, see FAQ section below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would leave this up to you to extend this MR with some additional descriptions.

| `keep_latest_nupkg_n_version(count=N)` | Leaves N nupkg (adds `*.nupkg` filter) in release feature builds |
| `keep_latest_n_file(count=N)` | Leaves the last (by creation time) files in the amount of N pieces. WITHOUT accounting subfolders |
| `keep_latest_n_file_in_folder(count=N)` | Leaves the last (by creation time) files in the number of N pieces in each folder |
Expand Down