Skip to content

Use PATHs to call clear_artifacts #4296

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 1 commit into from
Jun 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
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
20 changes: 19 additions & 1 deletion readthedocs/builds/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ def save(self, *args, **kwargs): # pylint: disable=arguments-differ
def delete(self, *args, **kwargs): # pylint: disable=arguments-differ
from readthedocs.projects import tasks
log.info('Removing files for version %s', self.slug)
broadcast(type='app', task=tasks.clear_artifacts, args=[self.pk])
broadcast(type='app', task=tasks.clear_artifacts, args=[self.get_artifact_paths()])
broadcast(
type='app', task=tasks.symlink_project, args=[self.project.pk])
super(Version, self).delete(*args, **kwargs)
Expand Down Expand Up @@ -237,6 +237,24 @@ def get_build_path(self):
return path
return None

def get_artifact_paths(self):
"""
Return a list of all production artifacts/media path for this version.

:rtype: list
"""
paths = []

for type_ in ('pdf', 'epub', 'htmlzip'):
paths.append(
self.project.get_production_media_path(
type_=type_,
version_slug=self.slug),
)
paths.append(self.project.rtd_build_path(version=self.slug))

return paths

def clean_build_path(self):
"""
Clean build path for project version.
Expand Down
62 changes: 22 additions & 40 deletions readthedocs/projects/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -764,10 +764,21 @@ def sync_files(project_pk, version_pk, hostname=None, html=False,
synchronization of build artifacts on each application instance.
"""
# Clean up unused artifacts
version = Version.objects.get(pk=version_pk)
if not pdf:
clear_pdf_artifacts(version_pk)
remove_dir(
version.project.get_production_media_path(
type_='pdf',
version_slug=version.slug,
),
)
if not epub:
clear_epub_artifacts(version_pk)
remove_dir(
version.project.get_production_media_path(
type_='epub',
version_slug=version.slug,
),
)

# Sync files to the web servers
move_files(
Expand All @@ -777,7 +788,7 @@ def sync_files(project_pk, version_pk, hostname=None, html=False,
localmedia=localmedia,
search=search,
pdf=pdf,
epub=epub
epub=epub,
)

# Symlink project
Expand Down Expand Up @@ -1158,44 +1169,15 @@ def remove_dir(path):


@app.task()
def clear_artifacts(version_pk):
"""Remove artifacts from the web servers."""
version = Version.objects.get(pk=version_pk)
clear_pdf_artifacts(version)
clear_epub_artifacts(version)
clear_htmlzip_artifacts(version)
clear_html_artifacts(version)


@app.task()
def clear_pdf_artifacts(version):
if isinstance(version, int):
version = Version.objects.get(pk=version)
remove_dir(version.project.get_production_media_path(
type_='pdf', version_slug=version.slug))


@app.task()
def clear_epub_artifacts(version):
if isinstance(version, int):
version = Version.objects.get(pk=version)
remove_dir(version.project.get_production_media_path(
type_='epub', version_slug=version.slug))


@app.task()
def clear_htmlzip_artifacts(version):
if isinstance(version, int):
version = Version.objects.get(pk=version)
remove_dir(version.project.get_production_media_path(
type_='htmlzip', version_slug=version.slug))

def clear_artifacts(paths):
Copy link
Member Author

Choose a reason for hiding this comment

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

I kept the name, but it could be changed to remove_dirs. Although, it won't reflect the specific usage that we give to this task.

"""
Remove artifacts from the web servers.

@app.task()
def clear_html_artifacts(version):
if isinstance(version, int):
version = Version.objects.get(pk=version)
remove_dir(version.project.rtd_build_path(version=version.slug))
:param paths: list containing PATHs where production media is on disk
(usually ``Version.get_artifact_paths``)
"""
for path in paths:
remove_dir(path)


@app.task(queue='web')
Expand Down
4 changes: 2 additions & 2 deletions readthedocs/projects/views/private.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ def project_version_detail(request, project_slug, version_slug):
if 'active' in form.changed_data and version.active is False:
log.info('Removing files for version %s', version.slug)
broadcast(
type='app', task=tasks.clear_artifacts, args=[version.pk])
type='app', task=tasks.clear_artifacts, args=[version.get_artifact_paths()])
version.built = False
version.save()
url = reverse('project_version_list', args=[project.slug])
Expand Down Expand Up @@ -649,7 +649,7 @@ def project_version_delete_html(request, project_slug, version_slug):
if not version.active:
version.built = False
version.save()
broadcast(type='app', task=tasks.clear_artifacts, args=[version.pk])
broadcast(type='app', task=tasks.clear_artifacts, args=[version.get_artifact_paths()])
else:
return HttpResponseBadRequest(
"Can't delete HTML for an active version.")
Expand Down
4 changes: 2 additions & 2 deletions readthedocs/rtd_tests/tests/test_celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,14 @@ def test_clear_artifacts(self):
directory = self.project.get_production_media_path(type_='pdf', version_slug=version.slug)
os.makedirs(directory)
self.assertTrue(exists(directory))
result = tasks.clear_artifacts.delay(version_pk=version.pk)
result = tasks.clear_artifacts.delay(paths=version.get_artifact_paths())
self.assertTrue(result.successful())
self.assertFalse(exists(directory))

directory = version.project.rtd_build_path(version=version.slug)
os.makedirs(directory)
self.assertTrue(exists(directory))
result = tasks.clear_artifacts.delay(version_pk=version.pk)
result = tasks.clear_artifacts.delay(paths=version.get_artifact_paths())
self.assertTrue(result.successful())
self.assertFalse(exists(directory))

Expand Down