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: cache get-app artifacts by commit_hash #1519

Merged
merged 12 commits into from
Jan 23, 2024
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# MAC OS
.DS_Store

# VS Code
.vscode/

# Vim Gitignore
## Swap
[._]*.s[a-v][a-z]
Expand Down
154 changes: 149 additions & 5 deletions bench/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
import shutil
import subprocess
import sys
import tarfile
import typing
from collections import OrderedDict
from datetime import date
from functools import lru_cache
from pathlib import Path
from urllib.parse import urlparse

# imports - third party imports
Expand All @@ -23,6 +25,7 @@
UNSET_ARG,
fetch_details_from_tag,
get_available_folder_name,
get_bench_cache_path,
is_bench_directory,
is_git_url,
is_valid_frappe_branch,
Expand Down Expand Up @@ -166,13 +169,15 @@ def __init__(
branch: str = None,
bench: "Bench" = None,
soft_link: bool = False,
cache_key = None,
*args,
**kwargs,
):
self.bench = bench
self.soft_link = soft_link
self.required_by = None
self.local_resolution = []
self.cache_key = cache_key
super().__init__(name, branch, *args, **kwargs)

@step(title="Fetching App {repo}", success="App {repo} Fetched")
Expand Down Expand Up @@ -227,6 +232,7 @@ def install(
resolved=False,
restart_bench=True,
ignore_resolution=False,
using_cached=False
):
import bench.cli
from bench.utils.app import get_app_name
Expand All @@ -247,6 +253,7 @@ def install(
skip_assets=skip_assets,
restart_bench=restart_bench,
resolution=self.local_resolution,
using_cached=using_cached,
)

@step(title="Cloning and installing {repo}", success="App {repo} Installed")
Expand Down Expand Up @@ -283,6 +290,134 @@ def update_app_state(self):
branch=self.tag,
required=self.local_resolution,
)


"""
Get App Cache

Since get-app affects only the `apps`, `env`, and `sites`
bench sub directories. If we assume deterministic builds
when get-app is called, the `apps/app_name` sub dir can be
cached.

In subsequent builds this would save time by not having to:
- clone repository
- install frontend dependencies
- building frontend assets
as all of this is contained in the `apps/app_name` sub dir.

Code that updates the `env` and `sites` subdirs still need
to be run.
"""

def get_app_path(self) -> Path:
return Path(self.bench.name) / "apps" / self.app_name

def get_app_cache_path(self, is_compressed=False) -> Path:
assert self.cache_key is not None

cache_path = get_bench_cache_path("apps")
ext = "tgz" if is_compressed else "tar"
tarfile_name = f"{self.app_name}-{self.cache_key[:10]}.{ext}"
return cache_path / tarfile_name

def get_cached(self) -> bool:
if not self.cache_key:
return False

cache_path = self.get_app_cache_path()
mode = "r"

# Check if cache exists without gzip
if not cache_path.is_file():
cache_path = self.get_app_cache_path(True)
mode = "r:gz"

# Check if cache exists with gzip
if not cache_path.is_file():
return False

app_path = self.get_app_path()
if app_path.is_dir():
shutil.rmtree(app_path)

click.secho(f"Getting {self.app_name} from cache", fg="yellow")
with tarfile.open(cache_path, mode) as tar:
tar.extractall(app_path.parent)

return True

def set_cache(self, compress_artifacts=False) -> bool:
if not self.cache_key:
return False

app_path = self.get_app_path()
if not app_path.is_dir():
return False

cwd = os.getcwd()
cache_path = self.get_app_cache_path(compress_artifacts)
mode = "w:gz" if compress_artifacts else "w"

message = f"Caching {self.app_name} app directory"
if compress_artifacts:
message += " (compressed)"
click.secho(message)

self.prune_app_directory()

success = False
os.chdir(app_path.parent)
Copy link
Member

@ankush ankush Jan 17, 2024

Choose a reason for hiding this comment

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

If possible avoid changing directories. This has caused problems in past as bench assumes a certain directory to be cwd always.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here it's required cause of how tar functions, it stores the entire path and untarring requires moving the file out of nested directories. I've wrapped tar in a try finally so cwd should be restored irrespective: 30f301e

try:
with tarfile.open(cache_path, mode) as tar:
tar.add(app_path.name)
success = True
except Exception:
log(f"Failed to cache {app_path}", level=3)
success = False
finally:
os.chdir(cwd)
return success

def prune_app_directory(self):
app_path = self.get_app_path()
remove_unused_node_modules(app_path)


def remove_unused_node_modules(app_path: Path) -> None:
"""
Erring a bit the side of caution; since there is no explicit way
to check if node_modules are utilized, this function checks if Vite
is being used to build the frontend code.

Since most popular Frappe apps use Vite to build their frontends,
this method should suffice.

Note: root package.json is ignored cause those usually belong to
apps that do not have a build step and so their node_modules are
utilized during runtime.
"""

for p in app_path.iterdir():
if not p.is_dir():
continue

package_json = p / "package.json"
if not package_json.is_file():
continue

node_modules = p / "node_modules"
if not node_modules.is_dir():
continue

can_delete = False
with package_json.open("r", encoding="utf-8") as f:
package_json = json.loads(f.read())
build_script = package_json.get("scripts", {}).get("build", "")
can_delete = "vite build" in build_script

if can_delete:
shutil.rmtree(node_modules)


def make_resolution_plan(app: App, bench: "Bench"):
Expand Down Expand Up @@ -346,6 +481,8 @@ def get_app(
soft_link=False,
init_bench=False,
resolve_deps=False,
cache_key=None,
compress_artifacts=False,
):
"""bench get-app clones a Frappe App from remote (GitHub or any other git server),
and installs it on the current bench. This also resolves dependencies based on the
Expand All @@ -360,14 +497,14 @@ def get_app(
from bench.utils.app import check_existing_dir

bench = Bench(bench_path)
app = App(git_url, branch=branch, bench=bench, soft_link=soft_link)
app = App(git_url, branch=branch, bench=bench, soft_link=soft_link, cache_key=cache_key)
git_url = app.url
repo_name = app.repo
branch = app.tag
bench_setup = False
restart_bench = not init_bench
frappe_path, frappe_branch = None, None

if resolve_deps:
resolution = make_resolution_plan(app, bench)
click.secho("Following apps will be installed", fg="bright_blue")
Expand Down Expand Up @@ -417,6 +554,10 @@ def get_app(
verbose=verbose,
)
return

if app.get_cached():
app.install(verbose=verbose, skip_assets=skip_assets, restart_bench=restart_bench, using_cached=True)
return

dir_already_exists, cloned_path = check_existing_dir(bench_path, repo_name)
to_clone = not dir_already_exists
Expand All @@ -442,6 +583,9 @@ def get_app(
or click.confirm("Do you want to reinstall the existing application?")
):
app.install(verbose=verbose, skip_assets=skip_assets, restart_bench=restart_bench)

app.set_cache(compress_artifacts)



def install_resolved_deps(
Expand All @@ -452,7 +596,6 @@ def install_resolved_deps(
verbose=False,
):
from bench.utils.app import check_existing_dir

if "frappe" in resolution:
# Terminal dependency
del resolution["frappe"]
Expand Down Expand Up @@ -550,6 +693,7 @@ def install_app(
restart_bench=True,
skip_assets=False,
resolution=UNSET_ARG,
using_cached=False,
):
import bench.cli as bench_cli
from bench.bench import Bench
Expand Down Expand Up @@ -577,14 +721,14 @@ def install_app(
if conf.get("developer_mode"):
install_python_dev_dependencies(apps=app, bench_path=bench_path, verbose=verbose)

if os.path.exists(os.path.join(app_path, "package.json")):
if not using_cached and os.path.exists(os.path.join(app_path, "package.json")):
yarn_install = "yarn install --verbose" if verbose else "yarn install"
bench.run(yarn_install, cwd=app_path)

bench.apps.sync(app_name=app, required=resolution, branch=tag, app_dir=app_path)

if not skip_assets:
build_assets(bench_path=bench_path, app=app)
build_assets(bench_path=bench_path, app=app, using_cached=using_cached)

if restart_bench:
# Avoiding exceptions here as production might not be set-up
Expand Down
2 changes: 2 additions & 0 deletions bench/commands/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ def bench_command(bench_path="."):


from bench.commands.utils import (
app_cache_helper,
backup_all_sites,
bench_src,
disable_production,
Expand Down Expand Up @@ -108,6 +109,7 @@ def bench_command(bench_path="."):
bench_command.add_command(bench_src)
bench_command.add_command(find_benches)
bench_command.add_command(migrate_env)
bench_command.add_command(app_cache_helper)

from bench.commands.setup import setup

Expand Down
16 changes: 16 additions & 0 deletions bench/commands/make.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,18 @@ def drop(path):
default=False,
help="Resolve dependencies before installing app",
)
@click.option(
"--cache-key",
type=str,
default=None,
help="Caches get-app artifacts if provided (only first 10 chars is used)",
)
@click.option(
"--compress-artifacts",
is_flag=True,
default=False,
help="Whether to gzip get-app artifacts that are to be cached",
)
def get_app(
git_url,
branch,
Expand All @@ -160,6 +172,8 @@ def get_app(
soft_link=False,
init_bench=False,
resolve_deps=False,
cache_key=None,
compress_artifacts=False,
):
"clone an app from the internet and set it up in your bench"
from bench.app import get_app
Expand All @@ -172,6 +186,8 @@ def get_app(
soft_link=soft_link,
init_bench=init_bench,
resolve_deps=resolve_deps,
cache_key=cache_key,
compress_artifacts=compress_artifacts,
)


Expand Down
18 changes: 18 additions & 0 deletions bench/commands/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,3 +176,21 @@ def migrate_env(python, backup=True):
from bench.utils.bench import migrate_env

migrate_env(python=python, backup=backup)


@click.command("app-cache", help="View or remove items belonging to bench get-app cache")
@click.option("--clear", is_flag=True, default=False, help="Remove all items")
@click.option(
"--remove-app",
default="",
help="Removes all items that match provided app name",
)
@click.option(
"--remove-key",
default="",
help="Removes all items that matches provided cache key",
)
def app_cache_helper(clear=False, remove_app="", remove_key=""):
from bench.utils.bench import cache_helper

cache_helper(clear, remove_app, remove_key)
12 changes: 11 additions & 1 deletion bench/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
import sys
from functools import lru_cache
from glob import glob
from pathlib import Path
from shlex import split
from typing import List, Tuple
from typing import List, Optional, Tuple

# imports - third party imports
import click
Expand Down Expand Up @@ -50,6 +51,15 @@ def is_frappe_app(directory: str) -> bool:

return bool(is_frappe_app)

def get_bench_cache_path(sub_dir: Optional[str]) -> Path:
relative_path = "~/.cache/bench"
if sub_dir and not sub_dir.startswith("/"):
relative_path += f"/{sub_dir}"

cache_path = os.path.expanduser(relative_path)
cache_path = Path(cache_path)
cache_path.mkdir(parents=True, exist_ok=True)
return cache_path

@lru_cache(maxsize=None)
def is_valid_frappe_branch(frappe_path: str, frappe_branch: str):
Expand Down
Loading