From fd1ebf5f71faf84cc35d1ad5199ddeaea9b9b7b7 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Sun, 22 Oct 2023 16:52:29 +1100 Subject: [PATCH] formatting and pylint fixes --- bookwyrm/importers/bookwyrm_import.py | 7 +-- bookwyrm/models/bookwyrm_export_job.py | 22 ++++---- bookwyrm/models/bookwyrm_import_job.py | 51 ++++++++++--------- bookwyrm/models/job.py | 20 ++++++-- bookwyrm/models/notification.py | 2 +- .../templates/settings/imports/imports.html | 6 +-- .../tests/models/test_bookwyrm_export_job.py | 5 +- .../tests/models/test_bookwyrm_import_job.py | 20 +++++--- .../views/preferences/test_export_user.py | 3 +- bookwyrm/utils/tar.py | 13 +++-- bookwyrm/views/preferences/export.py | 5 +- 11 files changed, 91 insertions(+), 63 deletions(-) diff --git a/bookwyrm/importers/bookwyrm_import.py b/bookwyrm/importers/bookwyrm_import.py index a2eb71725..c8f4433ca 100644 --- a/bookwyrm/importers/bookwyrm_import.py +++ b/bookwyrm/importers/bookwyrm_import.py @@ -1,14 +1,15 @@ """Import data from Bookwyrm export files""" -from bookwyrm import settings from bookwyrm.models.bookwyrm_import_job import BookwyrmImportJob class BookwyrmImporter: - """Import a Bookwyrm User export JSON file. + """Import a Bookwyrm User export file. This is kind of a combination of an importer and a connector. """ - def process_import(self, user, archive_file, settings): + def process_import( + self, user, archive_file, settings + ): # pylint: disable=no-self-use """import user data from a Bookwyrm export file""" required = [k for k in settings if settings.get(k) == "on"] diff --git a/bookwyrm/models/bookwyrm_export_job.py b/bookwyrm/models/bookwyrm_export_job.py index 65f209905..e3fb2a81f 100644 --- a/bookwyrm/models/bookwyrm_export_job.py +++ b/bookwyrm/models/bookwyrm_export_job.py @@ -1,4 +1,7 @@ +"""Export user account to tar.gz file for import into another Bookwyrm instance""" + import logging +from uuid import uuid4 from django.db.models import FileField from django.db.models import Q @@ -6,10 +9,9 @@ from django.core.serializers.json import DjangoJSONEncoder from django.core.files.base import ContentFile from bookwyrm import models +from bookwyrm.models.job import ParentJob, ParentTask from bookwyrm.settings import DOMAIN from bookwyrm.tasks import app, IMPORTS -from bookwyrm.models.job import ParentJob, ParentTask, SubTask, create_child_job -from uuid import uuid4 from bookwyrm.utils.tar import BookwyrmTarFile logger = logging.getLogger(__name__) @@ -49,21 +51,22 @@ def start_export_task(**kwargs): job.save(update_fields=["export_data"]) -def tar_export(json_data: str, user, f): - f.open("wb") - with BookwyrmTarFile.open(mode="w:gz", fileobj=f) as tar: +def tar_export(json_data: str, user, file): + """wrap the export information in a tar file""" + file.open("wb") + with BookwyrmTarFile.open(mode="w:gz", fileobj=file) as tar: tar.write_bytes(json_data.encode("utf-8")) # Add avatar image if present if getattr(user, "avatar", False): tar.add_image(user.avatar, filename="avatar") - editions, books = get_books_for_user(user) + editions = get_books_for_user(user) for book in editions: if getattr(book, "cover", False): tar.add_image(book.cover) - f.close() + file.close() def json_export(user): @@ -91,18 +94,19 @@ def json_export(user): # reading goals reading_goals = models.AnnualGoal.objects.filter(user=user).distinct() goals_list = [] + # TODO: either error checking should be more sophisticated or maybe we don't need this try/except try: for goal in reading_goals: goals_list.append( {"goal": goal.goal, "year": goal.year, "privacy": goal.privacy} ) - except Exception: + except Exception: # pylint: disable=broad-except pass try: readthroughs = models.ReadThrough.objects.filter(user=user).distinct().values() readthroughs = list(readthroughs) - except Exception as e: + except Exception: # pylint: disable=broad-except readthroughs = [] # books diff --git a/bookwyrm/models/bookwyrm_import_job.py b/bookwyrm/models/bookwyrm_import_job.py index 6e71aa4b5..8fe797ed7 100644 --- a/bookwyrm/models/bookwyrm_import_job.py +++ b/bookwyrm/models/bookwyrm_import_job.py @@ -1,3 +1,5 @@ +"""Import a user from another Bookwyrm instance""" + from functools import reduce import json import logging @@ -11,13 +13,7 @@ from django.contrib.postgres.fields import ArrayField as DjangoArrayField from bookwyrm import activitypub from bookwyrm import models from bookwyrm.tasks import app, IMPORTS -from bookwyrm.models.job import ( - ParentJob, - ParentTask, - ChildJob, - SubTask, - create_child_job, -) +from bookwyrm.models.job import ParentJob, ParentTask, SubTask from bookwyrm.utils.tar import BookwyrmTarFile logger = logging.getLogger(__name__) @@ -161,8 +157,10 @@ def get_or_create_edition(book_data, tar): if cover_path: tar.write_image_to_file(cover_path, new_book.cover) - # NOTE: clean_values removes "last_edited_by" because it's a user ID from the old database - # if this is required, bookwyrm_export_job will need to bring in the user who edited it. + # NOTE: clean_values removes "last_edited_by" + # because it's a user ID from the old database + # if this is required, bookwyrm_export_job will + # need to bring in the user who edited it. # create parent work = models.Work.objects.create(title=book["title"]) @@ -197,7 +195,7 @@ def clean_values(data): return new_data -def find_existing(cls, data, user): +def find_existing(cls, data): """Given a book or author, find any existing model instances""" identifiers = [ @@ -248,27 +246,31 @@ def upsert_readthroughs(data, user, book_id): """Take a JSON string of readthroughs, find or create the instances in the database and return a list of saved instances""" - for rt in data: + for read_thru in data: start_date = ( - parse_datetime(rt["start_date"]) if rt["start_date"] is not None else None + parse_datetime(read_thru["start_date"]) + if read_thru["start_date"] is not None + else None ) finish_date = ( - parse_datetime(rt["finish_date"]) if rt["finish_date"] is not None else None + parse_datetime(read_thru["finish_date"]) + if read_thru["finish_date"] is not None + else None ) stopped_date = ( - parse_datetime(rt["stopped_date"]) - if rt["stopped_date"] is not None + parse_datetime(read_thru["stopped_date"]) + if read_thru["stopped_date"] is not None else None ) readthrough = { "user": user, "book": models.Edition.objects.get(id=book_id), - "progress": rt["progress"], - "progress_mode": rt["progress_mode"], + "progress": read_thru["progress"], + "progress_mode": read_thru["progress_mode"], "start_date": start_date, "finish_date": finish_date, "stopped_date": stopped_date, - "is_active": rt["is_active"], + "is_active": read_thru["is_active"], } existing = models.ReadThrough.objects.filter(**readthrough).exists() @@ -311,7 +313,8 @@ def get_or_create_statuses(user, cls, data, book_id): def upsert_lists(user, lists, items, book_id): - """Take a list and ListItems as JSON and create DB entries if they don't already exist""" + """Take a list and ListItems as JSON and + create DB entries if they don't already exist""" book = models.Edition.objects.get(id=book_id) @@ -408,7 +411,7 @@ def update_user_settings(user, data): @app.task(queue=IMPORTS, base=SubTask) -def update_user_settings_task(job_id, child_id): +def update_user_settings_task(job_id): """wrapper task for user's settings import""" parent_job = BookwyrmImportJob.objects.get(id=job_id) @@ -433,7 +436,7 @@ def update_goals(user, data): @app.task(queue=IMPORTS, base=SubTask) -def update_goals_task(job_id, child_id): +def update_goals_task(job_id): """wrapper task for user's goals import""" parent_job = BookwyrmImportJob.objects.get(id=job_id) @@ -450,7 +453,7 @@ def upsert_saved_lists(user, values): @app.task(queue=IMPORTS, base=SubTask) -def upsert_saved_lists_task(job_id, child_id): +def upsert_saved_lists_task(job_id): """wrapper task for user's saved lists import""" parent_job = BookwyrmImportJob.objects.get(id=job_id) @@ -477,7 +480,7 @@ def upsert_follows(user, values): @app.task(queue=IMPORTS, base=SubTask) -def upsert_follows_task(job_id, child_id): +def upsert_follows_task(job_id): """wrapper task for user's follows import""" parent_job = BookwyrmImportJob.objects.get(id=job_id) @@ -504,7 +507,7 @@ def upsert_user_blocks(user, user_ids): @app.task(queue=IMPORTS, base=SubTask) -def upsert_user_blocks_task(job_id, child_id): +def upsert_user_blocks_task(job_id): """wrapper task for user's blocks import""" parent_job = BookwyrmImportJob.objects.get(id=job_id) diff --git a/bookwyrm/models/job.py b/bookwyrm/models/job.py index 4ba4bc2d7..4f5cb2093 100644 --- a/bookwyrm/models/job.py +++ b/bookwyrm/models/job.py @@ -31,6 +31,8 @@ class Job(models.Model): ) class Meta: + """Make it abstract""" + abstract = True def complete_job(self): @@ -119,7 +121,7 @@ class ParentJob(Job): if not self.complete and self.has_completed: self.complete_job() - def __terminate_job(self): + def __terminate_job(self): # pylint: disable=unused-private-member """Tell workers to ignore and not execute this task & pending child tasks. Extend. """ @@ -183,7 +185,9 @@ class ParentTask(app.Task): Usage e.g. @app.task(base=ParentTask) """ - def before_start(self, task_id, args, kwargs): + def before_start( + self, task_id, args, kwargs + ): # pylint: disable=no-self-use, unused-argument """Handler called before the task starts. Override. Prepare ParentJob before the task starts. @@ -208,7 +212,9 @@ class ParentTask(app.Task): if kwargs["no_children"]: job.set_status(ChildJob.Status.ACTIVE) - def on_success(self, retval, task_id, args, kwargs): + def on_success( + self, retval, task_id, args, kwargs + ): # pylint: disable=no-self-use, unused-argument """Run by the worker if the task executes successfully. Override. Update ParentJob on Task complete. @@ -241,7 +247,9 @@ class SubTask(app.Task): Usage e.g. @app.task(base=SubTask) """ - def before_start(self, task_id, args, kwargs): + def before_start( + self, task_id, args, kwargs + ): # pylint: disable=no-self-use, unused-argument """Handler called before the task starts. Override. Prepare ChildJob before the task starts. @@ -263,7 +271,9 @@ class SubTask(app.Task): child_job.save(update_fields=["task_id"]) child_job.set_status(ChildJob.Status.ACTIVE) - def on_success(self, retval, task_id, args, kwargs): + def on_success( + self, retval, task_id, args, kwargs + ): # pylint: disable=no-self-use, unused-argument """Run by the worker if the task executes successfully. Override. Notify ChildJob of task completion. diff --git a/bookwyrm/models/notification.py b/bookwyrm/models/notification.py index c8140bce9..98d20a3cb 100644 --- a/bookwyrm/models/notification.py +++ b/bookwyrm/models/notification.py @@ -1,6 +1,7 @@ """ alert a user to activity """ from django.db import models, transaction from django.dispatch import receiver +from bookwyrm.models.bookwyrm_export_job import BookwyrmExportJob from .base_model import BookWyrmModel from . import ( Boost, @@ -10,7 +11,6 @@ from . import ( BookwyrmImportJob, LinkDomain, ) -from bookwyrm.models.bookwyrm_export_job import BookwyrmExportJob from . import ListItem, Report, Status, User, UserFollowRequest diff --git a/bookwyrm/templates/settings/imports/imports.html b/bookwyrm/templates/settings/imports/imports.html index 09d12b04a..0f4ae04fc 100644 --- a/bookwyrm/templates/settings/imports/imports.html +++ b/bookwyrm/templates/settings/imports/imports.html @@ -141,7 +141,7 @@
- {% url 'settings-imports' status as url %} + {% url 'settings-imports' status as url %} @@ -231,7 +231,7 @@
{% trans "ID" %}
- {% url 'settings-imports' status as url %} + {% url 'settings-imports' status as url %} @@ -299,5 +299,5 @@ {% include 'snippets/pagination.html' with page=user_imports path=request.path %} - {% endblock %} +{% endblock %} diff --git a/bookwyrm/tests/models/test_bookwyrm_export_job.py b/bookwyrm/tests/models/test_bookwyrm_export_job.py index bd314e60e..73b59a4cc 100644 --- a/bookwyrm/tests/models/test_bookwyrm_export_job.py +++ b/bookwyrm/tests/models/test_bookwyrm_export_job.py @@ -1,3 +1,4 @@ +"""test bookwyrm user export functions""" import datetime import time import json @@ -110,7 +111,7 @@ class BookwyrmExport(TestCase): ) # add to list - item = models.ListItem.objects.create( + models.ListItem.objects.create( book_list=self.list, user=self.local_user, book=self.edition, @@ -226,7 +227,7 @@ class BookwyrmExport(TestCase): json_data["books"][0]["quotes"][0]["quote"], "A rose by any other name" ) - def test_tar_export(self): + def test_tar_export(self): # pylint: disable=unnecessary-pass """test the tar export function""" # TODO diff --git a/bookwyrm/tests/models/test_bookwyrm_import_job.py b/bookwyrm/tests/models/test_bookwyrm_import_job.py index 61713cd17..c07772e16 100644 --- a/bookwyrm/tests/models/test_bookwyrm_import_job.py +++ b/bookwyrm/tests/models/test_bookwyrm_import_job.py @@ -5,14 +5,12 @@ import pathlib from unittest.mock import patch from django.db.models import Q -from django.utils import timezone from django.utils.dateparse import parse_datetime from django.test import TestCase from bookwyrm import models -from bookwyrm.settings import DOMAIN from bookwyrm.utils.tar import BookwyrmTarFile -import bookwyrm.models.bookwyrm_import_job as bookwyrm_import_job +from bookwyrm.models import bookwyrm_import_job class BookwyrmImport(TestCase): @@ -246,7 +244,9 @@ class BookwyrmImport(TestCase): self.assertEqual(author.name, "James C. Scott") def test_get_or_create_edition_existing(self): - """Test take a JSON string of books and editions, find or create the editions in the database and return a list of edition instances""" + """Test take a JSON string of books and editions, + find or create the editions in the database and + return a list of edition instances""" self.assertEqual(models.Edition.objects.count(), 1) self.assertEqual(models.Edition.objects.count(), 1) @@ -258,7 +258,9 @@ class BookwyrmImport(TestCase): self.assertEqual(models.Edition.objects.count(), 1) def test_get_or_create_edition_not_existing(self): - """Test take a JSON string of books and editions, find or create the editions in the database and return a list of edition instances""" + """Test take a JSON string of books and editions, + find or create the editions in the database and + return a list of edition instances""" self.assertEqual(models.Edition.objects.count(), 1) @@ -441,7 +443,8 @@ class BookwyrmImport(TestCase): ) def test_upsert_list_existing(self): - """Take a list and ListItems as JSON and create DB entries if they don't already exist""" + """Take a list and ListItems as JSON and create DB entries + if they don't already exist""" book_data = self.import_data["books"][0] @@ -456,7 +459,7 @@ class BookwyrmImport(TestCase): name="my list of books", user=self.local_user ) - list_item = models.ListItem.objects.create( + models.ListItem.objects.create( book=self.book, book_list=book_list, user=self.local_user, order=1 ) @@ -489,7 +492,8 @@ class BookwyrmImport(TestCase): ) def test_upsert_list_not_existing(self): - """Take a list and ListItems as JSON and create DB entries if they don't already exist""" + """Take a list and ListItems as JSON and create DB entries + if they don't already exist""" book_data = self.import_data["books"][0] diff --git a/bookwyrm/tests/views/preferences/test_export_user.py b/bookwyrm/tests/views/preferences/test_export_user.py index 1483fc4ec..654ed2a05 100644 --- a/bookwyrm/tests/views/preferences/test_export_user.py +++ b/bookwyrm/tests/views/preferences/test_export_user.py @@ -1,5 +1,4 @@ -""" test for app action functionality """ -from collections import namedtuple +""" test for user export app functionality """ from unittest.mock import patch from django.http import HttpResponse diff --git a/bookwyrm/utils/tar.py b/bookwyrm/utils/tar.py index 448df48d9..8f43b2c15 100644 --- a/bookwyrm/utils/tar.py +++ b/bookwyrm/utils/tar.py @@ -1,12 +1,15 @@ +"""manage tar files for user exports""" +import io +import tarfile from uuid import uuid4 from django.core.files import File -import tarfile -import io class BookwyrmTarFile(tarfile.TarFile): - def write_bytes(self, data: bytes, filename="archive.json"): - """Add a file containing :data: bytestring with name :filename: to the archive""" + """Create tar files for user exports""" + + def write_bytes(self, data: bytes): + """Add a file containing bytes to the archive""" buffer = io.BytesIO(data) info = tarfile.TarInfo("archive.json") info.size = len(data) @@ -30,10 +33,12 @@ class BookwyrmTarFile(tarfile.TarFile): self.addfile(info, fileobj=image) def read(self, filename): + """read data from the tar""" with self.extractfile(filename) as reader: return reader.read() def write_image_to_file(self, filename, file_field): + """add an image to the tar""" extension = filename.rsplit(".")[-1] with self.extractfile(filename) as reader: filename = f"{str(uuid4())}.{extension}" diff --git a/bookwyrm/views/preferences/export.py b/bookwyrm/views/preferences/export.py index 037b8dbdc..c55e12c86 100644 --- a/bookwyrm/views/preferences/export.py +++ b/bookwyrm/views/preferences/export.py @@ -13,7 +13,7 @@ from django.views import View from django.utils.decorators import method_decorator from django.shortcuts import redirect -from bookwyrm import models, settings +from bookwyrm import models from bookwyrm.models.bookwyrm_export_job import BookwyrmExportJob from bookwyrm.settings import PAGE_LENGTH @@ -135,10 +135,11 @@ class ExportUser(View): @method_decorator(login_required, name="dispatch") -class ExportArchive(View): +class ExportArchive(View): # pylint: disable=line-too-long """Serve the archive file""" def get(self, request, archive_id): + """download user export file""" export = BookwyrmExportJob.objects.get(task_id=archive_id, user=request.user) return HttpResponse( export.export_data,
{% trans "ID" %}