From d17ceec1f00872113fea61c3f95a367444740094 Mon Sep 17 00:00:00 2001 From: Eliot Berriot <contact@eliotberriot.com> Date: Sun, 10 Jun 2018 12:06:46 +0200 Subject: [PATCH] See #297: linting of various, uncommon errors --- api/config/api_urls.py | 2 -- api/config/asgi.py | 2 +- api/config/routing.py | 1 - api/config/settings/common.py | 6 ++-- api/config/settings/local.py | 5 +-- api/config/settings/production.py | 2 -- api/config/urls.py | 1 - api/funkwhale_api/federation/actors.py | 2 +- api/funkwhale_api/federation/serializers.py | 3 +- api/funkwhale_api/federation/views.py | 4 +-- api/funkwhale_api/music/models.py | 12 +++---- api/funkwhale_api/music/tasks.py | 2 +- api/funkwhale_api/music/views.py | 2 +- .../providers/audiofile/tasks.py | 36 ------------------- api/funkwhale_api/radios/factories.py | 2 +- api/funkwhale_api/radios/filters.py | 1 - api/funkwhale_api/subsonic/views.py | 4 +-- api/funkwhale_api/users/models.py | 3 +- api/setup.cfg | 3 +- api/tests/data/youtube.py | 4 +-- api/tests/music/conftest.py | 2 +- api/tests/music/test_lyrics.py | 8 ++--- api/tests/music/test_serializers.py | 1 - api/tests/music/test_works.py | 3 +- api/tests/requests/test_models.py | 3 -- api/tests/subsonic/test_renderers.py | 2 +- api/tests/test_youtube.py | 4 +-- api/tests/users/test_permissions.py | 8 ++--- api/tests/users/test_views.py | 20 +---------- 29 files changed, 39 insertions(+), 109 deletions(-) diff --git a/api/config/api_urls.py b/api/config/api_urls.py index 9e634b7c..9f87a7af 100644 --- a/api/config/api_urls.py +++ b/api/config/api_urls.py @@ -1,12 +1,10 @@ from django.conf.urls import include, url from dynamic_preferences.api.viewsets import GlobalPreferencesViewSet -from dynamic_preferences.users.viewsets import UserPreferencesViewSet from rest_framework import routers from rest_framework.urlpatterns import format_suffix_patterns from rest_framework_jwt import views as jwt_views from funkwhale_api.activity import views as activity_views -from funkwhale_api.instance import views as instance_views from funkwhale_api.music import views from funkwhale_api.playlists import views as playlists_views from funkwhale_api.subsonic.views import SubsonicViewSet diff --git a/api/config/asgi.py b/api/config/asgi.py index df741d25..5ecc3ffa 100644 --- a/api/config/asgi.py +++ b/api/config/asgi.py @@ -2,7 +2,7 @@ import os import django -from .routing import application +from .routing import application # noqa os.environ.setdefault("DJANGO_SETTINGS_MODULE", "config.settings.production") diff --git a/api/config/routing.py b/api/config/routing.py index 07b5392d..fa25aad0 100644 --- a/api/config/routing.py +++ b/api/config/routing.py @@ -1,4 +1,3 @@ -from channels.auth import AuthMiddlewareStack from channels.routing import ProtocolTypeRouter, URLRouter from django.conf.urls import url diff --git a/api/config/settings/common.py b/api/config/settings/common.py index c39ca8ca..cb5573ed 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -11,7 +11,6 @@ https://docs.djangoproject.com/en/dev/ref/settings/ from __future__ import absolute_import, unicode_literals import datetime -import os from urllib.parse import urlparse, urlsplit import environ @@ -23,7 +22,6 @@ ROOT_DIR = environ.Path(__file__) - 3 # (/a/b/myfile.py - 3 = /) APPS_DIR = ROOT_DIR.path("funkwhale_api") env = environ.Env() - try: env.read_env(ROOT_DIR.file(".env")) except FileNotFoundError: @@ -333,12 +331,12 @@ CACHES["default"]["OPTIONS"] = { } -########## CELERY +# CELERY INSTALLED_APPS += ("funkwhale_api.taskapp.celery.CeleryConfig",) CELERY_BROKER_URL = env( "CELERY_BROKER_URL", default=env("CACHE_URL", default=CACHE_DEFAULT) ) -########## END CELERY +# END CELERY # Location of root django.contrib.admin URL, use {% url 'admin:index' %} # Your common stuff: Below this line define 3rd party library settings diff --git a/api/config/settings/local.py b/api/config/settings/local.py index 51e79347..9f0119ce 100644 --- a/api/config/settings/local.py +++ b/api/config/settings/local.py @@ -10,6 +10,7 @@ Local settings from .common import * # noqa + # DEBUG # ------------------------------------------------------------------------------ DEBUG = env.bool("DJANGO_DEBUG", default=True) @@ -49,10 +50,10 @@ INSTALLED_APPS += ("debug_toolbar",) # ------------------------------------------------------------------------------ TEST_RUNNER = "django.test.runner.DiscoverRunner" -########## CELERY +# CELERY # In development, all tasks will be executed locally by blocking until the task returns CELERY_TASK_ALWAYS_EAGER = False -########## END CELERY +# END CELERY # Your local stuff: Below this line define 3rd party library settings diff --git a/api/config/settings/production.py b/api/config/settings/production.py index b81eb8ad..72b08aa3 100644 --- a/api/config/settings/production.py +++ b/api/config/settings/production.py @@ -11,8 +11,6 @@ Production Configurations """ from __future__ import absolute_import, unicode_literals -from django.utils import six - from .common import * # noqa # SECRET CONFIGURATION diff --git a/api/config/urls.py b/api/config/urls.py index eb3f66ca..5ffcf211 100644 --- a/api/config/urls.py +++ b/api/config/urls.py @@ -6,7 +6,6 @@ from django.conf.urls import include, url from django.conf.urls.static import static from django.contrib import admin from django.views import defaults as default_views -from django.views.generic import TemplateView urlpatterns = [ # Django Admin, use {% url 'admin:index' %} diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index f0b6b098..7fbf815d 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -35,7 +35,7 @@ def get_actor_data(actor_url): response.raise_for_status() try: return response.json() - except: + except Exception: raise ValueError("Invalid actor payload: {}".format(response.text)) diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index a7063246..062f74f4 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -334,7 +334,7 @@ class FollowSerializer(serializers.Serializer): return models.Follow.objects.get_or_create( actor=self.validated_data["actor"], target=self.validated_data["object"], - **kwargs, + **kwargs, # noqa )[0] def to_representation(self, instance): @@ -345,7 +345,6 @@ class FollowSerializer(serializers.Serializer): "object": instance.target.url, "type": "Follow", } - return ret class APIFollowSerializer(serializers.ModelSerializer): diff --git a/api/funkwhale_api/federation/views.py b/api/funkwhale_api/federation/views.py index 7b013e12..63a1d7b7 100644 --- a/api/funkwhale_api/federation/views.py +++ b/api/funkwhale_api/federation/views.py @@ -1,7 +1,7 @@ from django import forms from django.core import paginator from django.db import transaction -from django.http import HttpResponse +from django.http import HttpResponse, Http404 from django.urls import reverse from rest_framework import mixins, response, viewsets from rest_framework.decorators import detail_route, list_route @@ -144,7 +144,7 @@ class MusicFilesViewSet(FederationMixin, viewsets.GenericViewSet): else: try: page_number = int(page) - except: + except Exception: return response.Response({"page": ["Invalid page number"]}, status=400) p = paginator.Paginator( qs, preferences.get("federation__collection_page_size") diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index 3ba3c126..8b638ce7 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -259,13 +259,13 @@ class Work(APIModelMixin): import_hooks = [import_lyrics, link_recordings] def fetch_lyrics(self): - l = self.lyrics.first() - if l: - return l + lyric = self.lyrics.first() + if lyric: + return lyric data = self.api.get(self.mbid, includes=["url-rels"])["work"] - l = import_lyrics(self, {}, data) + lyric = import_lyrics(self, {}, data) - return l + return lyric class Lyrics(models.Model): @@ -606,7 +606,7 @@ def update_request_status(sender, instance, created, **kwargs): if not instance.import_request: return - if not created and not "status" in update_fields: + if not created and "status" not in update_fields: return r_status = instance.import_request.status diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index 788ef5da..355af770 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/api/funkwhale_api/music/tasks.py @@ -256,7 +256,7 @@ def import_job_run(self, import_job, replace=False, use_acoustid=False): if not settings.DEBUG: try: self.retry(exc=exc, countdown=30, max_retries=3) - except: + except Exception: mark_errored(exc) raise mark_errored(exc) diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index 3f823b53..77a82dd2 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -4,7 +4,7 @@ import urllib from django.conf import settings from django.core.exceptions import ObjectDoesNotExist -from django.db import models, transaction +from django.db import transaction from django.db.models import Count from django.db.models.functions import Length from django.utils import timezone diff --git a/api/funkwhale_api/providers/audiofile/tasks.py b/api/funkwhale_api/providers/audiofile/tasks.py index 624e068c..ee486345 100644 --- a/api/funkwhale_api/providers/audiofile/tasks.py +++ b/api/funkwhale_api/providers/audiofile/tasks.py @@ -1,12 +1,6 @@ -import os - -import acoustid -from django.core.files import File from django.db import transaction from funkwhale_api.music import metadata, models -from funkwhale_api.providers.acoustid import get_acoustid_client -from funkwhale_api.taskapp import celery @transaction.atomic @@ -49,33 +43,3 @@ def import_track_data_from_path(path): defaults={"title": data.get("title"), "position": position}, )[0] return track - - -def import_metadata_with_musicbrainz(path): - pass - - -@celery.app.task(name="audiofile.from_path") -def from_path(path): - acoustid_track_id = None - try: - client = get_acoustid_client() - result = client.get_best_match(path) - acoustid_track_id = result["id"] - except acoustid.WebServiceError: - track = import_track_data_from_path(path) - except (TypeError, KeyError): - track = import_metadata_without_musicbrainz(path) - else: - track, created = models.Track.get_or_create_from_api( - mbid=result["recordings"][0]["id"] - ) - - if track.files.count() > 0: - raise ValueError("File already exists for track {}".format(track.pk)) - - track_file = models.TrackFile(track=track, acoustid_track_id=acoustid_track_id) - track_file.audio_file.save(os.path.basename(path), File(open(path, "rb"))) - track_file.save() - - return track_file diff --git a/api/funkwhale_api/radios/factories.py b/api/funkwhale_api/radios/factories.py index 69d6ab6e..a83c5373 100644 --- a/api/funkwhale_api/radios/factories.py +++ b/api/funkwhale_api/radios/factories.py @@ -24,7 +24,7 @@ class RadioSessionFactory(factory.django.DjangoModelFactory): @registry.register(name="radios.CustomRadioSession") -class RadioSessionFactory(factory.django.DjangoModelFactory): +class CustomRadioSessionFactory(factory.django.DjangoModelFactory): user = factory.SubFactory(UserFactory) radio_type = "custom" custom_radio = factory.SubFactory( diff --git a/api/funkwhale_api/radios/filters.py b/api/funkwhale_api/radios/filters.py index 158bf578..810673bd 100644 --- a/api/funkwhale_api/radios/filters.py +++ b/api/funkwhale_api/radios/filters.py @@ -175,7 +175,6 @@ class TagFilter(RadioFilter): "type": "list", "subtype": "string", "autocomplete": reverse_lazy("api:v1:tags-list"), - "autocomplete_qs": "", "autocomplete_fields": { "remoteValues": "results", "name": "name", diff --git a/api/funkwhale_api/subsonic/views.py b/api/funkwhale_api/subsonic/views.py index 09cd8663..7fb5db0e 100644 --- a/api/funkwhale_api/subsonic/views.py +++ b/api/funkwhale_api/subsonic/views.py @@ -456,6 +456,6 @@ class SubsonicViewSet(viewsets.GenericViewSet): {"error": {"code": 0, "message": "Invalid payload"}} ) if serializer.validated_data["submission"]: - l = serializer.save() - record.send(l) + listening = serializer.save() + record.send(listening) return response.Response({}) diff --git a/api/funkwhale_api/users/models.py b/api/funkwhale_api/users/models.py index d198ff9a..caf1e452 100644 --- a/api/funkwhale_api/users/models.py +++ b/api/funkwhale_api/users/models.py @@ -90,7 +90,8 @@ class User(AbstractUser): perms[p] = v return perms - def has_permissions(self, *perms, operator="and"): + def has_permissions(self, *perms, **kwargs): + operator = kwargs.pop("operator", "and") if operator not in ["and", "or"]: raise ValueError("Invalid operator {}".format(operator)) permissions = self.get_permissions() diff --git a/api/setup.cfg b/api/setup.cfg index 53e08c41..18e34bc3 100644 --- a/api/setup.cfg +++ b/api/setup.cfg @@ -1,6 +1,7 @@ [flake8] max-line-length = 120 -exclude = .tox,.git,*/migrations/*,*/static/CACHE/*,docs,node_modules +exclude = .tox,.git,*/migrations/*,*/static/CACHE/*,docs,node_modules,tests/data,tests/music/conftest.py +ignore = F405,W503,E203 [isort] skip_glob = .tox,.git,*/migrations/*,*/static/CACHE/*,docs,node_modules diff --git a/api/tests/data/youtube.py b/api/tests/data/youtube.py index d3303d2a..9c8f9e68 100644 --- a/api/tests/data/youtube.py +++ b/api/tests/data/youtube.py @@ -1,5 +1,3 @@ - - search = {} @@ -13,7 +11,7 @@ search["8 bit adventure"] = { "etag": '"gMxXHe-zinKdE9lTnzKu8vjcmDI/GxK-wHBWUYfrJsd1dijBPTufrVE"', "snippet": { "liveBroadcastContent": "none", - "description": "Make sure to apply adhesive evenly before use. GET IT HERE: http://adhesivewombat.bandcamp.com/album/marsupial-madness Facebook: ...", + "description": "Description", "channelId": "UCps63j3krzAG4OyXeEyuhFw", "title": "AdhesiveWombat - 8 Bit Adventure", "channelTitle": "AdhesiveWombat", diff --git a/api/tests/music/conftest.py b/api/tests/music/conftest.py index 634fb337..0cb6f477 100644 --- a/api/tests/music/conftest.py +++ b/api/tests/music/conftest.py @@ -617,7 +617,7 @@ def lyricswiki_content(): <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> - <meta name="viewport" content="width=device-width, initial-scale=1.0, user-scalable=yes"> +<meta name="viewport" content="width=device-width, initial-scale=1.0, user-scalable=yes"> <meta name="generator" content="MediaWiki 1.19.24" /> <meta name="keywords" content="Chop Suey! lyrics,System Of A Down Chop Suey! lyrics,Chop Suey! by System Of A Down lyrics,lyrics,LyricWiki,LyricWikia,lyricwiki,System Of A Down:Chop Suey!,System Of A Down,System Of A Down:Toxicity (2001),Enter Shikari,Enter Shikari:Chop Suey!,"Weird Al" Yankovic,"Weird Al" Yankovic:Angry White Boy Polka,Renard,Renard:Physicality,System Of A Down:Chop Suey!/pt,Daron Malakian" /> <meta name="description" content="Chop Suey! This song is by System of a Down and appears on the album Toxicity (2001)." /> diff --git a/api/tests/music/test_lyrics.py b/api/tests/music/test_lyrics.py index ad0c6afb..c8ce92b6 100644 --- a/api/tests/music/test_lyrics.py +++ b/api/tests/music/test_lyrics.py @@ -4,7 +4,7 @@ from funkwhale_api.music import lyrics as lyrics_utils from funkwhale_api.music import models, tasks -def test_works_import_lyrics_if_any(lyricswiki_content, mocker, factories): +def test_lyrics_tasks(lyricswiki_content, mocker, factories): mocker.patch( "funkwhale_api.music.lyrics._get_html", return_value=lyricswiki_content ) @@ -14,7 +14,7 @@ def test_works_import_lyrics_if_any(lyricswiki_content, mocker, factories): tasks.fetch_content(lyrics_id=lyrics.pk) lyrics.refresh_from_db() - self.assertIn("Grab a brush and put on a little makeup", lyrics.content) + assert "Grab a brush and put on a little makeup" in lyrics.content def test_clean_content(): @@ -32,10 +32,10 @@ def test_markdown_rendering(factories): content = """Hello Is it me you're looking for?""" - l = factories["music.Lyrics"](content=content) + lyrics = factories["music.Lyrics"](content=content) expected = "<p>Hello<br />\nIs it me you're looking for?</p>" - assert expected == l.content_rendered + assert expected == lyrics.content_rendered def test_works_import_lyrics_if_any( diff --git a/api/tests/music/test_serializers.py b/api/tests/music/test_serializers.py index 9402e145..51ca96b5 100644 --- a/api/tests/music/test_serializers.py +++ b/api/tests/music/test_serializers.py @@ -63,7 +63,6 @@ def test_track_file_serializer(factories, to_api_date): "path": tf.path, "source": tf.source, "filename": tf.filename, - "mimetype": tf.mimetype, "track": tf.track.pk, "duration": tf.duration, "mimetype": tf.mimetype, diff --git a/api/tests/music/test_works.py b/api/tests/music/test_works.py index bd69c8ab..96b537ca 100644 --- a/api/tests/music/test_works.py +++ b/api/tests/music/test_works.py @@ -1,4 +1,3 @@ - from funkwhale_api.music import models @@ -36,7 +35,7 @@ def test_can_get_work_from_recording(factories, mocker, works, tracks): ) mbid = "e2ecabc4-1b9d-30b2-8f30-3596ec423dc5" - assert recording.work == None + assert recording.work is None work = recording.get_work() diff --git a/api/tests/requests/test_models.py b/api/tests/requests/test_models.py index f23fc9ef..3ac8a534 100644 --- a/api/tests/requests/test_models.py +++ b/api/tests/requests/test_models.py @@ -1,6 +1,3 @@ - - - def test_can_bind_import_batch_to_request(factories): request = factories["requests.ImportRequest"]() diff --git a/api/tests/subsonic/test_renderers.py b/api/tests/subsonic/test_renderers.py index 65dae6f2..301fee8b 100644 --- a/api/tests/subsonic/test_renderers.py +++ b/api/tests/subsonic/test_renderers.py @@ -24,7 +24,7 @@ def test_xml_renderer_dict_to_xml(): def test_xml_renderer(): payload = {"hello": "world"} - expected = b'<?xml version="1.0" encoding="UTF-8"?>\n<subsonic-response hello="world" status="ok" version="1.16.0" xmlns="http://subsonic.org/restapi" />' + expected = b'<?xml version="1.0" encoding="UTF-8"?>\n<subsonic-response hello="world" status="ok" version="1.16.0" xmlns="http://subsonic.org/restapi" />' # noqa renderer = renderers.SubsonicXMLRenderer() rendered = renderer.render(payload) diff --git a/api/tests/test_youtube.py b/api/tests/test_youtube.py index aa3b8a05..cb5559ce 100644 --- a/api/tests/test_youtube.py +++ b/api/tests/test_youtube.py @@ -33,7 +33,7 @@ def test_can_get_search_results_from_funkwhale(preferences, mocker, api_client, "id": "0HxZn6CzOIo", "url": "https://www.youtube.com/watch?v=0HxZn6CzOIo", "type": "youtube#video", - "description": "Make sure to apply adhesive evenly before use. GET IT HERE: http://adhesivewombat.bandcamp.com/album/marsupial-madness Facebook: ...", + "description": "Description", "channelId": "UCps63j3krzAG4OyXeEyuhFw", "title": "AdhesiveWombat - 8 Bit Adventure", "channelTitle": "AdhesiveWombat", @@ -82,7 +82,7 @@ def test_can_send_multiple_queries_at_once_from_funwkhale( "id": "0HxZn6CzOIo", "url": "https://www.youtube.com/watch?v=0HxZn6CzOIo", "type": "youtube#video", - "description": "Make sure to apply adhesive evenly before use. GET IT HERE: http://adhesivewombat.bandcamp.com/album/marsupial-madness Facebook: ...", + "description": "Description", "channelId": "UCps63j3krzAG4OyXeEyuhFw", "title": "AdhesiveWombat - 8 Bit Adventure", "channelTitle": "AdhesiveWombat", diff --git a/api/tests/users/test_permissions.py b/api/tests/users/test_permissions.py index f4b50101..7f72138f 100644 --- a/api/tests/users/test_permissions.py +++ b/api/tests/users/test_permissions.py @@ -87,8 +87,6 @@ def test_has_user_permission_logged_in_multiple_or( request = api_request.get("/") setattr(request, "user", user) result = permission.has_permission(request, view) - assert ( - result - == user.has_permissions("federation", "library", operator="or") - == expected - ) + has_permission_result = user.has_permissions("federation", "library", operator="or") + + assert result == has_permission_result == expected diff --git a/api/tests/users/test_views.py b/api/tests/users/test_views.py index 10320f8e..00272c2a 100644 --- a/api/tests/users/test_views.py +++ b/api/tests/users/test_views.py @@ -161,7 +161,7 @@ def test_user_can_request_new_subsonic_token(logged_in_api_client): assert response.data == {"subsonic_api_token": user.subsonic_api_token} -def test_user_can_get_new_subsonic_token(logged_in_api_client): +def test_user_can_get_subsonic_token(logged_in_api_client): user = logged_in_api_client.user user.subsonic_api_token = "test" user.save() @@ -176,24 +176,6 @@ def test_user_can_get_new_subsonic_token(logged_in_api_client): assert response.data == {"subsonic_api_token": "test"} -def test_user_can_request_new_subsonic_token(logged_in_api_client): - user = logged_in_api_client.user - user.subsonic_api_token = "test" - user.save() - - url = reverse( - "api:v1:users:users-subsonic-token", kwargs={"username": user.username} - ) - - response = logged_in_api_client.post(url) - - assert response.status_code == 200 - user.refresh_from_db() - assert user.subsonic_api_token != "test" - assert user.subsonic_api_token is not None - assert response.data == {"subsonic_api_token": user.subsonic_api_token} - - def test_user_can_delete_subsonic_token(logged_in_api_client): user = logged_in_api_client.user user.subsonic_api_token = "test" -- GitLab