From cba905e367ad8b6f7b835692532b7253c9d19432 Mon Sep 17 00:00:00 2001 From: Jonas L Date: Fri, 28 Jan 2022 06:09:19 +0100 Subject: [PATCH] refactor(analyzer): improve analyzer pipeline module (#1542) * rename steps to pipeline module * move pipeline entrypoint to pipeline module * rename steps test module to pipeline * fix paths after renames * move step protocol to pipeline * create pipeline status enum * use Protocol from typing extensions * Fix linting --- .../libretime_analyzer/message_listener.py | 4 +-- .../libretime_analyzer/pipeline/__init__.py | 1 + .../{steps => pipeline}/analyze_cuepoint.py | 0 .../{steps => pipeline}/analyze_metadata.py | 0 .../analyze_playability.py | 0 .../{steps => pipeline}/analyze_replaygain.py | 0 .../{steps => pipeline}/organise_file.py | 0 .../{ => pipeline}/pipeline.py | 29 ++++++++++++++----- analyzer/libretime_analyzer/steps/step.py | 7 ----- analyzer/setup.py | 3 +- .../steps => tests/pipeline}/__init__.py | 0 .../analyze_cuepoint_test.py | 2 +- .../analyze_metadata_test.py | 2 +- .../analyze_playability_test.py | 4 +-- .../analyze_replaygain_test.py | 2 +- .../{steps => pipeline}/organise_file_test.py | 4 +-- .../tests/{ => pipeline}/pipeline_test.py | 2 +- analyzer/tests/steps/__init__.py | 0 18 files changed, 34 insertions(+), 26 deletions(-) create mode 100644 analyzer/libretime_analyzer/pipeline/__init__.py rename analyzer/libretime_analyzer/{steps => pipeline}/analyze_cuepoint.py (100%) rename analyzer/libretime_analyzer/{steps => pipeline}/analyze_metadata.py (100%) rename analyzer/libretime_analyzer/{steps => pipeline}/analyze_playability.py (100%) rename analyzer/libretime_analyzer/{steps => pipeline}/analyze_replaygain.py (100%) rename analyzer/libretime_analyzer/{steps => pipeline}/organise_file.py (100%) rename analyzer/libretime_analyzer/{ => pipeline}/pipeline.py (87%) delete mode 100644 analyzer/libretime_analyzer/steps/step.py rename analyzer/{libretime_analyzer/steps => tests/pipeline}/__init__.py (100%) rename analyzer/tests/{steps => pipeline}/analyze_cuepoint_test.py (88%) rename analyzer/tests/{steps => pipeline}/analyze_metadata_test.py (95%) rename analyzer/tests/{steps => pipeline}/analyze_playability_test.py (88%) rename analyzer/tests/{steps => pipeline}/analyze_replaygain_test.py (89%) rename analyzer/tests/{steps => pipeline}/organise_file_test.py (95%) rename analyzer/tests/{ => pipeline}/pipeline_test.py (95%) delete mode 100644 analyzer/tests/steps/__init__.py diff --git a/analyzer/libretime_analyzer/message_listener.py b/analyzer/libretime_analyzer/message_listener.py index d6915c0d1..c00636e6a 100644 --- a/analyzer/libretime_analyzer/message_listener.py +++ b/analyzer/libretime_analyzer/message_listener.py @@ -8,7 +8,7 @@ import pika from libretime_shared.config import RabbitMQConfig from loguru import logger -from .pipeline import Pipeline +from .pipeline import Pipeline, PipelineStatus from .status_reporter import StatusReporter EXCHANGE = "airtime-uploads" @@ -265,7 +265,7 @@ class MessageListener: metadata = q.get() except Exception as e: logger.error("Analyzer pipeline exception: %s" % str(e)) - metadata["import_status"] = Pipeline.IMPORT_STATUS_FAILED + metadata["import_status"] = PipelineStatus.failed # Ensure our queue doesn't fill up and block due to unexpected behaviour. Defensive code. while not q.empty(): diff --git a/analyzer/libretime_analyzer/pipeline/__init__.py b/analyzer/libretime_analyzer/pipeline/__init__.py new file mode 100644 index 000000000..18798d367 --- /dev/null +++ b/analyzer/libretime_analyzer/pipeline/__init__.py @@ -0,0 +1 @@ +from .pipeline import Pipeline, PipelineStatus diff --git a/analyzer/libretime_analyzer/steps/analyze_cuepoint.py b/analyzer/libretime_analyzer/pipeline/analyze_cuepoint.py similarity index 100% rename from analyzer/libretime_analyzer/steps/analyze_cuepoint.py rename to analyzer/libretime_analyzer/pipeline/analyze_cuepoint.py diff --git a/analyzer/libretime_analyzer/steps/analyze_metadata.py b/analyzer/libretime_analyzer/pipeline/analyze_metadata.py similarity index 100% rename from analyzer/libretime_analyzer/steps/analyze_metadata.py rename to analyzer/libretime_analyzer/pipeline/analyze_metadata.py diff --git a/analyzer/libretime_analyzer/steps/analyze_playability.py b/analyzer/libretime_analyzer/pipeline/analyze_playability.py similarity index 100% rename from analyzer/libretime_analyzer/steps/analyze_playability.py rename to analyzer/libretime_analyzer/pipeline/analyze_playability.py diff --git a/analyzer/libretime_analyzer/steps/analyze_replaygain.py b/analyzer/libretime_analyzer/pipeline/analyze_replaygain.py similarity index 100% rename from analyzer/libretime_analyzer/steps/analyze_replaygain.py rename to analyzer/libretime_analyzer/pipeline/analyze_replaygain.py diff --git a/analyzer/libretime_analyzer/steps/organise_file.py b/analyzer/libretime_analyzer/pipeline/organise_file.py similarity index 100% rename from analyzer/libretime_analyzer/steps/organise_file.py rename to analyzer/libretime_analyzer/pipeline/organise_file.py diff --git a/analyzer/libretime_analyzer/pipeline.py b/analyzer/libretime_analyzer/pipeline/pipeline.py similarity index 87% rename from analyzer/libretime_analyzer/pipeline.py rename to analyzer/libretime_analyzer/pipeline/pipeline.py index 230c57c36..f8c6a1e5d 100644 --- a/analyzer/libretime_analyzer/pipeline.py +++ b/analyzer/libretime_analyzer/pipeline/pipeline.py @@ -1,14 +1,29 @@ """ Analyzes and imports an audio file into the Airtime library. """ +from enum import Enum from queue import Queue +from typing import Any, Dict from loguru import logger +from typing_extensions import Protocol -from .steps.analyze_cuepoint import analyze_cuepoint -from .steps.analyze_metadata import analyze_metadata -from .steps.analyze_playability import UnplayableFileError, analyze_playability -from .steps.analyze_replaygain import analyze_replaygain -from .steps.organise_file import organise_file +from .analyze_cuepoint import analyze_cuepoint +from .analyze_metadata import analyze_metadata +from .analyze_playability import UnplayableFileError, analyze_playability +from .analyze_replaygain import analyze_replaygain +from .organise_file import organise_file + + +class Step(Protocol): + @staticmethod + def __call__(filename: str, metadata: Dict[str, Any]): + ... + + +class PipelineStatus(int, Enum): + succeed = 0 + pending = 1 + failed = 2 class Pipeline: @@ -21,8 +36,6 @@ class Pipeline: the failure to import can be reported back to the web application. """ - IMPORT_STATUS_FAILED = 2 - @staticmethod def run_analysis( queue, @@ -99,7 +112,7 @@ class Pipeline: queue.put(metadata) except UnplayableFileError as e: logger.exception(e) - metadata["import_status"] = Pipeline.IMPORT_STATUS_FAILED + metadata["import_status"] = PipelineStatus.failed metadata["reason"] = "The file could not be played." raise e except Exception as e: diff --git a/analyzer/libretime_analyzer/steps/step.py b/analyzer/libretime_analyzer/steps/step.py deleted file mode 100644 index 447264bb6..000000000 --- a/analyzer/libretime_analyzer/steps/step.py +++ /dev/null @@ -1,7 +0,0 @@ -from typing import Any, Dict, Protocol - - -class Step(Protocol): - @staticmethod - def __call__(filename: str, metadata: Dict[str, Any]): - ... diff --git a/analyzer/setup.py b/analyzer/setup.py index c50b77282..f7f7e2d43 100644 --- a/analyzer/setup.py +++ b/analyzer/setup.py @@ -21,7 +21,7 @@ setup( license="AGPLv3", packages=[ "libretime_analyzer", - "libretime_analyzer.steps", + "libretime_analyzer.pipeline", ], entry_points={ "console_scripts": [ @@ -34,6 +34,7 @@ setup( "pika>=1.0.0", "file-magic", "requests>=2.7.0", + "typing_extensions", ], extras_require={ "dev": [ diff --git a/analyzer/libretime_analyzer/steps/__init__.py b/analyzer/tests/pipeline/__init__.py similarity index 100% rename from analyzer/libretime_analyzer/steps/__init__.py rename to analyzer/tests/pipeline/__init__.py diff --git a/analyzer/tests/steps/analyze_cuepoint_test.py b/analyzer/tests/pipeline/analyze_cuepoint_test.py similarity index 88% rename from analyzer/tests/steps/analyze_cuepoint_test.py rename to analyzer/tests/pipeline/analyze_cuepoint_test.py index 46151425d..928354d0b 100644 --- a/analyzer/tests/steps/analyze_cuepoint_test.py +++ b/analyzer/tests/pipeline/analyze_cuepoint_test.py @@ -1,6 +1,6 @@ import pytest -from libretime_analyzer.steps.analyze_cuepoint import analyze_cuepoint +from libretime_analyzer.pipeline.analyze_cuepoint import analyze_cuepoint from ..fixtures import FILES diff --git a/analyzer/tests/steps/analyze_metadata_test.py b/analyzer/tests/pipeline/analyze_metadata_test.py similarity index 95% rename from analyzer/tests/steps/analyze_metadata_test.py rename to analyzer/tests/pipeline/analyze_metadata_test.py index 246e1a25d..a5646434f 100644 --- a/analyzer/tests/steps/analyze_metadata_test.py +++ b/analyzer/tests/pipeline/analyze_metadata_test.py @@ -1,6 +1,6 @@ import pytest -from libretime_analyzer.steps.analyze_metadata import analyze_metadata +from libretime_analyzer.pipeline.analyze_metadata import analyze_metadata from ..fixtures import FILE_INVALID_DRM, FILE_INVALID_TXT, FILES_TAGGED diff --git a/analyzer/tests/steps/analyze_playability_test.py b/analyzer/tests/pipeline/analyze_playability_test.py similarity index 88% rename from analyzer/tests/steps/analyze_playability_test.py rename to analyzer/tests/pipeline/analyze_playability_test.py index 17dc07587..52642af07 100644 --- a/analyzer/tests/steps/analyze_playability_test.py +++ b/analyzer/tests/pipeline/analyze_playability_test.py @@ -3,7 +3,7 @@ from unittest.mock import patch import distro import pytest -from libretime_analyzer.steps.analyze_playability import ( +from libretime_analyzer.pipeline.analyze_playability import ( UnplayableFileError, analyze_playability, ) @@ -21,7 +21,7 @@ def test_analyze_playability(filepath): def test_analyze_playability_missing_liquidsoap(): with patch( - "libretime_analyzer.steps.analyze_playability.LIQUIDSOAP_EXECUTABLE", + "libretime_analyzer.pipeline.analyze_playability.LIQUIDSOAP_EXECUTABLE", "foobar", ): analyze_playability(str(FILES[0].path), dict()) diff --git a/analyzer/tests/steps/analyze_replaygain_test.py b/analyzer/tests/pipeline/analyze_replaygain_test.py similarity index 89% rename from analyzer/tests/steps/analyze_replaygain_test.py rename to analyzer/tests/pipeline/analyze_replaygain_test.py index 527deee10..dd71d3aa7 100644 --- a/analyzer/tests/steps/analyze_replaygain_test.py +++ b/analyzer/tests/pipeline/analyze_replaygain_test.py @@ -1,7 +1,7 @@ import distro import pytest -from libretime_analyzer.steps.analyze_replaygain import analyze_replaygain +from libretime_analyzer.pipeline.analyze_replaygain import analyze_replaygain from ..fixtures import FILES diff --git a/analyzer/tests/steps/organise_file_test.py b/analyzer/tests/pipeline/organise_file_test.py similarity index 95% rename from analyzer/tests/steps/organise_file_test.py rename to analyzer/tests/pipeline/organise_file_test.py index 33c9934f0..2b2551fed 100644 --- a/analyzer/tests/steps/organise_file_test.py +++ b/analyzer/tests/pipeline/organise_file_test.py @@ -5,7 +5,7 @@ from unittest import mock import pytest -from libretime_analyzer.steps.organise_file import organise_file +from libretime_analyzer.pipeline.organise_file import organise_file from ..conftest import AUDIO_FILENAME @@ -82,7 +82,7 @@ def test_organise_file_triplicate_file(src_dir, dest_dir): # Here we use mock to patch out the time.localtime() function so that it # always returns the same value. This allows us to consistently simulate this test cases # where the last two of the three files are imported at the same time as the timestamp. - with mock.patch("libretime_analyzer.steps.organise_file.time") as mock_time: + with mock.patch("libretime_analyzer.pipeline.organise_file.time") as mock_time: mock_time.localtime.return_value = time.localtime() # date(2010, 10, 8) mock_time.side_effect = time.time diff --git a/analyzer/tests/pipeline_test.py b/analyzer/tests/pipeline/pipeline_test.py similarity index 95% rename from analyzer/tests/pipeline_test.py rename to analyzer/tests/pipeline/pipeline_test.py index a6efec0d2..3a23fc3f9 100644 --- a/analyzer/tests/pipeline_test.py +++ b/analyzer/tests/pipeline/pipeline_test.py @@ -6,7 +6,7 @@ import pytest from libretime_analyzer.pipeline import Pipeline -from .conftest import AUDIO_FILENAME, AUDIO_IMPORT_DEST +from ..conftest import AUDIO_FILENAME, AUDIO_IMPORT_DEST def test_run_analysis(src_dir, dest_dir): diff --git a/analyzer/tests/steps/__init__.py b/analyzer/tests/steps/__init__.py deleted file mode 100644 index e69de29bb..000000000