Skip to content

Commit 30f5a9e

Browse files
[FEATURE] Drive Identification and Refactoring (automatic-ripping-machine#1259)
* Add support for more device info USB devices frequently change their mount points on system reboot, causing faulty job associations to drives. To prevent this, drive recognition based on pyudev serial information (maker, model, serial number) of optical drives is introduced. This allows us to always associate a physical drive to the drives stored in the database without relying on fixed /dev/sr* numbers. On system startup, drives are now always scanned to recognize hardware changes. For backward compatibility, the stored serial is used primarily to identify the drive, and the mount point is only used as a fallback if the identification fails. This change also makes the drive name non-editable by the user. We are now also able to mark stale drives that were previously attached but are not found by the current pyudev run. This commit also addresses the unreliable eject status that was stored in the database. - If the drive was ejected (e.g. before system start) without the database being updated, this drive status became outdated. We have also added an image to illustrate that a drive was not found and was marked as stale in the settings overview. We reworked the Drive Settings Overview to use collapsible accordion items of bootstrap card items. The icons are now (to some extend) scalable instead of fixed size. The layout of the drive tab is now 2 column based to give the icon some space to extend to the bottom. This commit also addresses some of the pylint errors, introduces dataclasses for pyudev object handling and aims for better code readability. * Enhance MakeMKV Calls with Parser and Error Handling With this update, we've improved our MakeMKV integration by introducing a better parser that handles errors more robustly and provides greater clarity. The module is now also Pylint compatible, making it easier to maintain. To reduce noise in the logs, we've removed links to stdout logfiles, instead directing error reports through Python's built-in logger. This change reduces the file size of logs for users on info log level or higher. Additionally, when an internal error occurs that is likely to be expected due to a known failure, it will be marked as critical. These Changes will allow further improvements on working with the MakeMKV output. We introduce modern python concepts like dataclass and enums. * Raise Version to 2.16 * Handle MakeMKV info calls MakeMKV info calls create zombie processes and syslog errors at MakeMKV level. This changes help to reduce the amount of disturbing MakeMKV info calls. - cache MakeMKV disc number for disc:9999 calls - wait for MakeMKV processes to finish before calling info. - Give other MakeMKV processes time to call info before advancing in processing. This helps if multiple discs are inserted at the same time but waits for makemkv ripping to finish before calling info. - Update job status to reflect better, at which stage we are. * Maintainablility: Rename class A field should not duplicate the name of its containing class python:S1700 * Maintainability: Remove unnecessary pass Nested blocks of code should not be left empty python:S108 * Fix flake8 compatibility * Add python 3.6 compatibility * Reduce Function Complexity * Upgrade Process for Multiple Database Upgrades The current implementation assumed a commit-by-commit upgrade. As we reasonable cannot assume this, the system drives cannot get updated when the database updates as the table may have other pending changes that conflict with the function call. * Alter logging output. A lot of the sleep check messages are spammed to the log file. Reduce Messages by half the amount while preserving the information. After the sleep check was exited. A message is also added on info level that the job now continues. * Add more job status messages The job status messages are extended to show in the UI which process is stuck at waiting. * Consistency: remove button role Use <button> instead of the button role to ensure accessibility across all devices. * Refactor main makemkv function. Refactor this function to reduce its Cognitive Complexity from 17 to the 15 allowed. * Cleanup: Too many return statements Reduce the amount of return statements for better readability. * Messages to stdout as default Instead of duplicating this literal "--messages=-stdout" 5 times, we set it as default for makemkv in the run() function since we also direct stdout to a piped buffer in this function. It should be safe to assume that makemkv always accepts this cli parameter for all calls. * Remove label css class for non-inputs A form label must be associated with a control. Label elements should have a text label and an associated control Web:S6853 This applies for all fields that are not changeable by the user. * Add job status messages as variables Use only a defined set of status messages for the ripping process instead of the current various different (hard to track) status messages. * Add config option for concurrent MakeMKV info This setting turns on/off the limit for concurrent makemkv info calls to avoid crashes. * Fix: location of sleep * Keep Job Status Updates Where They Happen Move the job status updates to the makemkv info call. This ensures readability of status updates so it becomes clear who changes them. * Readability: Fix equality check. Previous type checks suggest that operands are int but the dataclass input is not type safe. To not confuse the reader we do a type conversion to int and then to bool to convert "1" to True. * Remove ToDo Make the ToDo an info message. It won't fix that easy and is not of much importance to store makemkv info persistent. * Merge this if statement with the enclosing one. Mergeable "if" statements should be combined python:S1066 * Adjust wait time Some processes continue early and leave makemkv info processes behind. Setting the wait time to higher values for finished processes should allow waiting processes to better pick up the slot. * docstrings * Add more Error Messages Add some more potentially fatal error messages. * Add traceback to logged exceptions For better error reporting, include the trace back in the log output. * BugFix: inverse logic * Add known error to tray status queries If a drive was recently detached (without calling scan drives), the error is known and the drive status should return None. * Order Drives by description This will allow the user to define the ordering of the drives in the settings page. E.g. adding 1st usb drive, 2nd drive, internal drive will order the drives accordingly. * Handle Not-Existing Drives in eject page The eject page did show poor error messages if the drive was not found in the database. This should help the user to get notified that we could not eject the requested drive. * BugFix: Don't use str.upper() If the connection information could not get retrieved, we do not expect string output here. Let jinja filter handle upper conversion and do not put a string if we have None here. * Use Enum for CDS (IOctl Drive Status) Use an Enum to aggregate the possible outputs of the Kernel drive status into readable output. Use tooltips to show this to the user. Make the drive status a callable function instead of a (magic) property to indicate that this function call might take some time and is not pure state conversion (which is what properties should do). * Fix pylint Error * DB Drive relationship in job is read only To avoid conflicts and tackle the SAWarning, we explicitly mark job.drive as read only. The drive is associated by setting job_id_current. * Tackle encoding for None values Since we are using .get to fill up the pyudev content, there might be a None value here and there. To make the string conversions fail safe, we check for this. * BugFix: TackID messed up * BugFix: Cancel Wait * Add eject with tray toggle to drive utils Handing over the logic to "eject -T" for toggling the tray open/close. Prevent opening the tray if a job is in progress and associated to a drive. * BugFix: Preserve Previous Job ID The Previous Job ID gets deleted when a new job is added and the job_finished method had been called prior. * Get drive debug info back Add the drive debug information (previously drive_status_debug) back to the output. * Preserve Job Association on Database deletion If a job in the database is deleted, we cannot know for sure if the association between job.devpath and job_id_previous is actually the right choice here. It may be that the server restarted in-between deletions and the mounts have changed. It may also be that the current job is still running during deletions. In general, this call adds unnecessary obfuscation to where the job_id_previous is actually changed. Adding a better (Exception) message to the job info page if a job is requested but missing and hiding unassociated job ids completley from the drive settings page. * Remove Redundant Clutter We want to check for an associated job id and can simply check if the Database association exists by using the None object. This should handle these cases similarly. * Handle pyudev calls during running MakeMKV If MakeMKV is running, pyudev does not report a proper drive. We do not want to mark these drives as stale and handle deletion of properties as the last step. * Add indicator for closed drive that is processing Indicate a running job on the drive by adding a green cycle wheel. * Add ripping finished ready criterion for drives The drive status is only updated for "success" and "fail" status. However, the drive is free to use after the Ripping process finished. This is currently whenever we go to the transcoding step. Some rework of the state logic is needed in v3 but this is good for now. Issue automatic-ripping-machine#1236 * Readability * Remove obfuscating log message * Allow int input to sleep_check_process For better readability, allow integer intput to sleep_check_process. This converts the time range passed to random to a single value output. * Fix flake8 and pylint Errors flake8: - fix spaces around variable pylint: - disable exception for no-member - fix bad-exception-cause * Add serial_id field to Database Make the drive name user editable and add a serial_id field to the database that contains the static information. Use name for user-space messages (e.g. flash) and serial_id for the logger. * Mask the serial Number to the User Frontend The last digits of the serial number is masked here. It is a good practice from the MakeMKV forum to not leave your serial number out in the open. The logs, however, still contain the unmasked numbers for unique identifiers with serial_id. * Update Documentation for new Drive Information Include the newly added fields into the drive information wiki page. * Mask Serial Number in Dataclass repr The Serial Number is not something we want to distribute with the logs. * BugFix: sqlalchemy backref sqlalchemy backrefs needs only be defined in one place. The way it was done, created 3 independent backrefs which was a bit confusing and not working properly when updating one of them. We do not need a backref for the last job association since the job does not know if it is the current or previous job for the drive. Only the job needs to know, what drive it was assigned to. This simplifies the backrefs to just one (active drive <--> active job) instead of three. * Only delete mdisc association at program start With the current implementation, mdisc could get deleted by the UI settings "scan for drives" operation. If this is done in-between a rip that requires the makemkvcon disc association, the job will fail due to missing mdisc id. Only deleting the mdisc once on UI startup ensures that it will get filled by the first call to makemkvcon info and not get emptied afterwards. * Remove explicit session.add of changed entities A session needs to get informed only on newly created entities. Having the session.add() in places where we changed an entity is not necessary. * Be less strict about when ripping Do update all mount points in the database with mdisc ids. This should ensure that the mdisc id is stored across all devices. * Update Progressbar instead of redrawing * Check for associated drive when ejecting Sometimes the drive might have been disassociated from the job by other means and thus cannot get ejected here. Check this and log the case. * Add more known Errors from MakeMKV Add the error code for a failure in disc opening. This probably means that the disc is beeing read by another process. We differentiate between fatal errors which raise (MakeMKV exit status is unreliably sometimes) and errors which are only directed to a higher log level to get user awareness. * Organize Possible Job Status and changes. Created an enum for JobState to keep better track of the different possible job states throughout the project. It also prevents new jobs to get created when any ripping (audio/video) is active. A job is not released when its status changes. The associated drive keeps track if the drive can be safely re-used by another job. Issue automatic-ripping-machine#1411 * Prevent too frequent updates of mdisc If multiple jobs call this piece of code too frequently after each other, the database will be locked. Using a deferred write here to limit the number of database writes during mdisc update. * Define a constant instead of duplicating literals The literal "route_settings.settings" was used multiple times. String literals should not be duplicated python:S1192 * Reduce Cognitive Complexity of makemkv function. The cognitive complexity for check_output and track_info was too high. Using a class based approach here to distinguish between possible error states. --------- Co-authored-by: Mtech <62650032+microtechno9000@users.noreply.github.com>
1 parent cdc7c1b commit 30f5a9e

28 files changed

Lines changed: 2974 additions & 708 deletions

VERSION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
2.16.1
1+
2.17.0
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
"""Extended Optical Drive Info
2+
3+
Revision ID: 2986d3f7ecf9
4+
Revises: 6870a5546912
5+
Create Date: 2024-11-27 10:15:32.678332
6+
7+
"""
8+
from alembic import op
9+
import sqlalchemy as sa
10+
11+
# pylint: disable=no-member
12+
13+
# revision identifiers, used by Alembic.
14+
revision = '2986d3f7ecf9'
15+
down_revision = '6870a5546912'
16+
branch_labels = None
17+
depends_on = None
18+
19+
20+
def upgrade():
21+
op.drop_column('system_drives', 'type') # can be generated
22+
op.drop_column('system_drives', 'open') # cannot be treated as stateless
23+
# new static information
24+
op.add_column('system_drives', sa.Column('maker', sa.String(25)))
25+
op.add_column('system_drives', sa.Column('model', sa.String(50)))
26+
op.add_column('system_drives', sa.Column('serial', sa.String(25)))
27+
op.add_column('system_drives', sa.Column('connection', sa.String(5)))
28+
op.add_column('system_drives', sa.Column('read_cd', sa.Boolean))
29+
op.add_column('system_drives', sa.Column('read_dvd', sa.Boolean))
30+
op.add_column('system_drives', sa.Column('read_bd', sa.Boolean))
31+
# new dynamic information
32+
op.add_column('system_drives', sa.Column('firmware', sa.String(10)))
33+
op.add_column('system_drives', sa.Column('location', sa.String(255)))
34+
op.add_column('system_drives', sa.Column('stale', sa.Boolean))
35+
36+
37+
def downgrade():
38+
op.add_column('system_drives', sa.Column('type', sa.String(20)))
39+
op.add_column('system_drives', sa.Column('open', sa.Column(sa.Boolean)))
40+
op.drop_column('system_drives', 'maker')
41+
op.drop_column('system_drives', 'model')
42+
op.drop_column('system_drives', 'serial')
43+
op.drop_column('system_drives', 'connection')
44+
op.drop_column('system_drives', 'read_cd')
45+
op.drop_column('system_drives', 'read_dvd')
46+
op.drop_column('system_drives', 'read_bd')
47+
op.drop_column('system_drives', 'firmware')
48+
op.drop_column('system_drives', 'location')
49+
op.drop_column('system_drives', 'stale')
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
"""Store MakeMKV disc id
2+
3+
Revision ID: 5ceacc391408
4+
Revises: 2986d3f7ecf9
5+
Create Date: 2024-12-05 22:38:52.709850
6+
7+
"""
8+
from alembic import op
9+
import sqlalchemy as sa
10+
11+
# pylint: disable=no-member
12+
13+
# revision identifiers, used by Alembic.
14+
revision = '5ceacc391408'
15+
down_revision = '2986d3f7ecf9'
16+
branch_labels = None
17+
depends_on = None
18+
19+
20+
def upgrade():
21+
op.add_column('system_drives', sa.Column('mdisc', sa.SmallInteger()))
22+
23+
24+
def downgrade():
25+
op.drop_column('system_drives', 'mdisc')
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
"""Add drive id field
2+
3+
Revision ID: a79af75f4b31
4+
Revises: 5ceacc391408
5+
Create Date: 2025-01-04 14:39:09.972294
6+
7+
"""
8+
from alembic import op
9+
import sqlalchemy as sa
10+
11+
# pylint: disable=no-member
12+
13+
# revision identifiers, used by Alembic.
14+
revision = 'a79af75f4b31'
15+
down_revision = '5ceacc391408'
16+
branch_labels = None
17+
depends_on = None
18+
19+
20+
def upgrade():
21+
# new static information
22+
op.add_column('system_drives', sa.Column('serial_id', sa.String(100)))
23+
24+
25+
def downgrade():
26+
op.drop_column('system_drives', 'serial_id')

arm/models/__init__.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,12 @@
1-
#!/usr/bin/env python3
1+
"""Database Models
2+
"""
3+
4+
from .alembic_version import AlembicVersion # noqa F401
5+
from .config import Config # noqa F401
6+
from .job import Job, JobState # noqa F401
7+
from .notifications import Notifications # noqa F401
8+
from .system_drives import SystemDrives # noqa F401
9+
from .system_info import SystemInfo # noqa F401
10+
from .track import Track # noqa F401
11+
from .ui_settings import UISettings # noqa F401
12+
from .user import User # noqa F401

arm/models/job.py

Lines changed: 125 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
1+
import enum
12
import logging
23
import os
34
import psutil
45
import pyudev
56
import subprocess
67
import time
78

9+
from datetime import datetime as dt
810
from prettytable import PrettyTable
11+
from sqlalchemy.ext.hybrid import hybrid_property
12+
913
from arm.ripper import music_brainz
1014
from arm.ui import db
1115
import arm.config.config as cfg
@@ -15,6 +19,67 @@
1519
from arm.models.config import Config # noqa: F401
1620

1721

22+
class JobState(str, enum.Enum):
23+
"""Possible states for Job.status.
24+
25+
The origin of the states is getting unclear at this point. Therefore, the
26+
possible `Job.status` states are defined as fixed enums to handle and group
27+
them better. Some come from CD, some from DVD ripping.
28+
29+
Note: The timestamps could also be saved particularily for each step to
30+
show, for example, the pure transcoding time without the waiting
31+
time.
32+
"""
33+
34+
# Job Finished States
35+
SUCCESS = "success"
36+
FAILURE = "fail"
37+
38+
# Manual wait (see job.config.MANUAL_WAIT)
39+
MANUAL_WAIT_STARTED = "waiting"
40+
41+
# Job Initialized or Pending
42+
IDLE = "active"
43+
"""An Idle Job may proceed to ripping or to finished.
44+
45+
- When initializing a job, the job is set to active
46+
- After Handbrake finishes, Job is set to active
47+
- After ABCD finishes, Job is set to active
48+
"""
49+
50+
# Video Ripping States
51+
VIDEO_RIPPING = "ripping"
52+
"""Indicate that makemkv is ripping."""
53+
VIDEO_WAITING = "waiting"
54+
"""Indicate that the job waits for user input or for the next queue slot."""
55+
VIDEO_INFO = "info"
56+
"""Indicate that the job calls makemkv info"""
57+
58+
# Audio ripping states
59+
AUDIO_RIPPING = "ripping"
60+
61+
# Transcoding states
62+
TRANSCODE_ACTIVE = "transcoding"
63+
TRANSCODE_WAITING = "waiting_transcode"
64+
65+
66+
JOB_STATUS_FINISHED = {
67+
JobState.SUCCESS,
68+
JobState.FAILURE,
69+
}
70+
JOB_STATUS_RIPPING = {
71+
JobState.AUDIO_RIPPING,
72+
JobState.VIDEO_RIPPING,
73+
JobState.MANUAL_WAIT_STARTED, # <-- not ripping, but undistinguishable
74+
JobState.VIDEO_WAITING,
75+
JobState.VIDEO_INFO,
76+
}
77+
JOB_STATUS_TRANSCODING = {
78+
JobState.TRANSCODE_ACTIVE,
79+
JobState.TRANSCODE_WAITING,
80+
}
81+
82+
1883
class Job(db.Model):
1984
"""
2085
Job Class hold most of the details for each job
@@ -28,6 +93,12 @@ class Job(db.Model):
2893
stop_time = db.Column(db.DateTime)
2994
job_length = db.Column(db.String(12))
3095
status = db.Column(db.String(32))
96+
"""Now that we have JobState, we should migrate this column.
97+
status = db.Column(
98+
db.Enum(JobState, name="job_state_enum", native_enum=False, validate_strings=True),
99+
nullable=False
100+
)
101+
"""
31102
stage = db.Column(db.String(63))
32103
no_of_titles = db.Column(db.Integer)
33104
title = db.Column(db.String(256))
@@ -211,21 +282,60 @@ def get_d(self):
211282
return return_dict
212283

213284
def eject(self):
214-
"""Eject disc if it hasn't previously been ejected"""
285+
"""Eject disc if it hasn't previously been ejected
286+
"""
287+
if self.ejected:
288+
logging.debug("The drive associated with this job has already been ejected.")
289+
return
290+
if self.drive is None:
291+
logging.warning("No drive was backpopulated with this job!")
292+
return
215293
if not cfg.arm_config['AUTO_EJECT']:
216294
logging.info("Skipping auto eject")
295+
self.drive.release_current_job()
217296
return
218-
if not self.ejected:
219-
self.ejected = True
220-
try:
221-
# This might always return true
222-
if bool(os.system("umount " + self.devpath)):
223-
logging.debug(f"Unmounted disc {self.devpath}")
224-
else:
225-
logging.debug(f"Failed to unmount {self.devpath}")
226-
if bool(os.system("eject -sv " + self.devpath)):
227-
logging.debug(f"Ejected disc {self.devpath}")
228-
else:
229-
logging.debug(f"Failed to eject {self.devpath}")
230-
except Exception as error:
231-
logging.debug(f"{self.devpath} couldn't be ejected {error}")
297+
if (error := self.drive.eject(method="eject", logger=logging)) is not None:
298+
logging.debug(f"{self.devpath} couldn't be ejected: {error}")
299+
self.ejected = True
300+
301+
@hybrid_property
302+
def finished(self):
303+
return JobState(self.status) in JOB_STATUS_FINISHED
304+
305+
@finished.expression
306+
def finished(cls):
307+
return cls.status.in_([js.value for js in JOB_STATUS_FINISHED])
308+
309+
@property
310+
def idle(self):
311+
return JobState(self.status) == JobState.IDLE
312+
313+
@property
314+
def ripping(self):
315+
return JobState(self.status) in JOB_STATUS_RIPPING
316+
317+
@property
318+
def run_time(self):
319+
return abs(dt.now() - self.start_time).total_seconds()
320+
321+
@property
322+
def ripping_finished(self):
323+
"""Indicates that the ripping process has finished.
324+
325+
Note: This usually means that we are transcoding and the drive is not
326+
currently used.
327+
"""
328+
if self.finished:
329+
logging.info("Job is finished.")
330+
return True
331+
if not self.ripping:
332+
logging.info("Job is not ripping.")
333+
return True
334+
if self.drive is None:
335+
logging.info("No drive was backpopulated with this job!")
336+
return True
337+
if self.ejected:
338+
logging.info(f"Drive {self.devpath} was ejected. No ripping process active.")
339+
return True
340+
logging.info(f"Job is ripping {self.devpath}.")
341+
return False

0 commit comments

Comments
 (0)