diff --git a/access/views.py b/access/views.py index a47f7fc..f306d6a 100644 --- a/access/views.py +++ b/access/views.py @@ -191,6 +191,7 @@ def aplus_json(request: HttpRequest, course_key: str) -> HttpResponse: except ConfigError as e: errors.append(f"Failed to load newly built course due to this error: {e}") errors.append("Attempting to load previous version of the course...") + logger.warn(f"Failed to load newly built course due to this error: {e}") else: defaults_path = CourseConfig.defaults_path(course_key, source=ConfigSource.STORE) @@ -198,6 +199,7 @@ def aplus_json(request: HttpRequest, course_key: str) -> HttpResponse: try: config = CourseConfig.get(course_key, source=ConfigSource.PUBLISH) except ConfigError as e: + logger.error(f"aplus_json: failed to get config for {course_key}") try: Course.objects.get(key=course_key) except: diff --git a/builder/builder.py b/builder/builder.py index 7e34d9e..534c661 100644 --- a/builder/builder.py +++ b/builder/builder.py @@ -28,7 +28,7 @@ from access.config import INDEX, ConfigSource, CourseConfig, load_meta, META from access.parser import ConfigError from builder.configure import configure_graders, publish_graders -from util.files import is_subpath, renames, rm_path, FileLock +from util.files import is_subpath, renames, rm_path, rm_paths, FileLock from util.git import checkout, clone_if_doesnt_exist, get_commit_hash, get_commit_metadata from util.pydantic import validation_error_str, validation_warning_str from util.static import static_url, static_url_path, symbolic_link @@ -336,14 +336,16 @@ def publish(course_key: str) -> List[str]: config = None errors = [] + tmpfiles = [] if Path(store_path).exists(): with FileLock(store_path): try: config = CourseConfig.get(course_key, source=ConfigSource.STORE) except ConfigError as e: errors.append(f"Failed to load newly built course for this reason: {e}") + logger.warn(f"Failed to load newly built course for this reason: {e}") else: - renames([ + tmpfiles = renames([ (store_path, prod_path), (store_defaults_path, prod_defaults_path), (store_version_path, prod_version_path), @@ -356,6 +358,7 @@ def publish(course_key: str) -> List[str]: config = CourseConfig.get(course_key, source=ConfigSource.PUBLISH) except ConfigError as e: errors.append(f"Failed to load already published config: {e}") + logger.error(f"Failed to load already published config: {e}") if config is None: if errors: @@ -364,8 +367,13 @@ def publish(course_key: str) -> List[str]: raise Exception(f"Course directory not found for {course_key} - the course probably has not been built") symbolic_link(config) + errors = errors + publish_graders(config) - return errors + publish_graders(config) + # Remove temporary files that were created during renaming. + # This may block the execution for a while on courses with many files. + rm_paths(tmpfiles) + + return errors # the task locks can get stuck if the program suddenly shuts down, diff --git a/util/files.py b/util/files.py index d52e1ff..b3cdfcb 100644 --- a/util/files.py +++ b/util/files.py @@ -9,7 +9,7 @@ import tempfile import time from types import TracebackType -from typing import Dict, Generator, Iterable, Optional, Tuple, Type, Union +from typing import Dict, Generator, Iterable, Optional, Tuple, Type, Union, List from django.conf import settings from django.http.response import FileResponse as DjangoFileResponse, HttpResponse @@ -46,6 +46,12 @@ def rm_path(path: Union[str, Path]) -> None: path.unlink() +def rm_paths(paths: Iterable[Union[str, Path]]) -> None: + for path in paths: + if path is not None: + rm_path(path) + + def is_subpath(child: PathLike, parent: Optional[PathLike] = None) -> bool: """ If parent is not None, returns whether child is a subpath of (contained in) @@ -168,9 +174,10 @@ def rename(src: PathLike, dst: PathLike, keep_tmp=False) -> Optional[str]: return tmpdst -def renames(pairs: Iterable[Tuple[PathLike, PathLike]]) -> None: +def renames(pairs: Iterable[Tuple[PathLike, PathLike]]) -> List[str]: """ Renames multiple files and directories while making sure that either all or none succeed. + Returns a list of generated tmp directories (for later removal). """ done = set() try: @@ -183,10 +190,8 @@ def renames(pairs: Iterable[Tuple[PathLike, PathLike]]) -> None: if os.path.exists(tmp): rename(tmp, dst) raise - else: - for _, _, tmp in done: - if tmp is not None: - rm_path(tmp) + + return list(map(lambda x: x[2], done)) def _try_lockf(lockfile, flags) -> Optional[OSError]: