Skip to content

Do not fail when unlinking an non-existing path #4760

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

Merged
merged 2 commits into from
Oct 16, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
16 changes: 8 additions & 8 deletions readthedocs/core/symlink.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@

from readthedocs.builds.models import Version
from readthedocs.core.utils.extend import SettingsOverrideObject
from readthedocs.core.utils import safe_makedirs
from readthedocs.core.utils import safe_makedirs, unlink
Copy link
Member

@ericholscher ericholscher Oct 16, 2018

Choose a reason for hiding this comment

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

Should this be called safe_unlink to mirror the safe_makedirs and show it has a bit more error handling?

from readthedocs.projects import constants
from readthedocs.projects.models import Domain
from readthedocs.projects.utils import run
Expand Down Expand Up @@ -95,7 +95,7 @@ def sanity_check(self):
log.info(constants.LOG_TEMPLATE.format(
project=self.project.slug, version='',
msg="Removing single version symlink"))
os.unlink(self.project_root)
unlink(self.project_root)
safe_makedirs(self.project_root)
elif (self.project.single_version and
not os.path.islink(self.project_root) and
Expand Down Expand Up @@ -164,7 +164,7 @@ def remove_symlink_cname(self, domain):
log.info(constants.LOG_TEMPLATE.format(project=self.project.slug,
version='', msg=log_msg))
symlink = os.path.join(self.CNAME_ROOT, domain.domain)
os.unlink(symlink)
unlink(symlink)

def symlink_subprojects(self):
"""
Expand Down Expand Up @@ -210,7 +210,7 @@ def symlink_subprojects(self):
if os.path.exists(self.subproject_root):
for subproj in os.listdir(self.subproject_root):
if subproj not in subprojects:
os.unlink(os.path.join(self.subproject_root, subproj))
unlink(os.path.join(self.subproject_root, subproj))

def symlink_translations(self):
"""
Expand All @@ -227,7 +227,7 @@ def symlink_translations(self):
# Make sure the language directory is a directory
language_dir = os.path.join(self.project_root, self.project.language)
if os.path.islink(language_dir):
os.unlink(language_dir)
unlink(language_dir)
if not os.path.lexists(language_dir):
safe_makedirs(language_dir)

Expand All @@ -246,7 +246,7 @@ def symlink_translations(self):
lang not in ['projects', self.project.language]):
to_delete = os.path.join(self.project_root, lang)
if os.path.islink(to_delete):
os.unlink(to_delete)
unlink(to_delete)
else:
shutil.rmtree(to_delete)

Expand All @@ -262,7 +262,7 @@ def symlink_single_version(self):
# Clean up symlinks
symlink = self.project_root
if os.path.islink(symlink):
os.unlink(symlink)
unlink(symlink)
if os.path.exists(symlink):
shutil.rmtree(symlink)

Expand Down Expand Up @@ -300,7 +300,7 @@ def symlink_versions(self):
if os.path.exists(version_dir):
for old_ver in os.listdir(version_dir):
if old_ver not in versions:
os.unlink(os.path.join(version_dir, old_ver))
unlink(os.path.join(version_dir, old_ver))

def get_default_version(self):
"""Look up project default version, return None if not found."""
Expand Down
17 changes: 17 additions & 0 deletions readthedocs/core/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,3 +225,20 @@ def safe_makedirs(directory_name):
if e.errno == errno.EEXIST:
pass
raise


def unlink(path):
"""
Unlink ``path`` symlink using ``os.unlink``.

This helper handles the exception ``FileNotFoundError`` to avoid logging in
cases where the symlink does not exist already and there is nothing to
unlink.

:param path: symlink path to unlink
:type path: str
"""
try:
os.unlink(path)
except FileNotFoundError:
log.warning('Unlink failed. Path %s does not exists', path)
4 changes: 2 additions & 2 deletions readthedocs/projects/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
from readthedocs.config import ConfigError
from readthedocs.core.resolver import resolve_path
from readthedocs.core.symlink import PublicSymlink, PrivateSymlink
from readthedocs.core.utils import send_email, broadcast
from readthedocs.core.utils import send_email, broadcast, unlink
from readthedocs.doc_builder.config import load_yaml_config
from readthedocs.doc_builder.constants import DOCKER_LIMITS
from readthedocs.doc_builder.environments import (
Expand Down Expand Up @@ -973,7 +973,7 @@ def remove_orphan_symlinks():
for cname in orphan_cnames:
orphan_domain_path = os.path.join(domain_path, cname)
log.info('Unlinking orphan CNAME: %s', orphan_domain_path)
os.unlink(orphan_domain_path)
unlink(orphan_domain_path)


@app.task(queue='web')
Expand Down