From d08871506dd4556b7d5fffca52ed6d13ca863f32 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simon=20K=C3=BCnzel?= <simonk@fsmpi.rwth-aachen.de>
Date: Sat, 12 Oct 2024 19:57:04 +0200
Subject: [PATCH] Fix bugs in media process
---
src/videoag_common/database/drift_detector.py | 14 ++++-
src/videoag_common/media_process/__init__.py | 2 +-
.../media_process/media_process.py | 17 +++++++
src/videoag_common/miscellaneous/json.py | 13 ++++-
src/videoag_common/objects/course.py | 2 +-
src/videoag_common/objects/medium.py | 51 +++++++++++++++++--
6 files changed, 88 insertions(+), 11 deletions(-)
diff --git a/src/videoag_common/database/drift_detector.py b/src/videoag_common/database/drift_detector.py
index 059af8e..4025e77 100644
--- a/src/videoag_common/database/drift_detector.py
+++ b/src/videoag_common/database/drift_detector.py
@@ -96,6 +96,10 @@ def _check_constraint_equal(actual_constraint: Constraint, schema_constraint: Co
and actual_constraint.name != schema_constraint.name):
return False
+ if actual_constraint.deferrable != schema_constraint.deferrable:
+ # TODO add to to_string
+ return False
+
if not isinstance(schema_constraint, ColumnCollectionConstraint):
print(f"Unknown type of constraint {schema_constraint}. Can't compare")
return False
@@ -113,13 +117,19 @@ def _check_constraint_equal(actual_constraint: Constraint, schema_constraint: Co
if not isinstance(actual_constraint, CheckConstraint):
return False
- # We try our best to check if they are equal
+ # We try our best to check if they are equal, but with things like 'x IN ()' it breaks (because the db
+ # transforms the statements)
if schema_constraint.sqltext == actual_constraint.sqltext:
return True
# Sometimes the db returns it without correct brackets. Just strip them
if str(schema_constraint.sqltext).strip("()") == str(actual_constraint.sqltext).strip("()"):
return True
- return False
+
+ print(f"Warning: Can't compare schema clause of constraint '{schema_constraint.name}'\n"
+ f" {schema_constraint.sqltext}\n"
+ f" with clause in database\n"
+ f" {actual_constraint.sqltext}")
+ return True
case ForeignKeyConstraint():
assert isinstance(schema_constraint, ForeignKeyConstraint) # For pycharm
diff --git a/src/videoag_common/media_process/__init__.py b/src/videoag_common/media_process/__init__.py
index d45dacf..1d4cff6 100644
--- a/src/videoag_common/media_process/__init__.py
+++ b/src/videoag_common/media_process/__init__.py
@@ -5,4 +5,4 @@ from .basic_targets import (
SourceFileTargetProducer,
DownscaleVideoTarget,
)
-from .media_process import MediaProcess, ApiMediaProcessField
+from .media_process import MediaProcess, ApiMediaProcessField, get_permanent_lecture_dir
diff --git a/src/videoag_common/media_process/media_process.py b/src/videoag_common/media_process/media_process.py
index 88ed5aa..1909220 100644
--- a/src/videoag_common/media_process/media_process.py
+++ b/src/videoag_common/media_process/media_process.py
@@ -3,12 +3,29 @@ from collections.abc import Iterable
from videoag_common.miscellaneous import *
from videoag_common.database import *
from videoag_common.api_object import ApiSimpleColumnField, FieldContext
+import videoag_common
from .target import TargetProducer, TARGET_ID_PATTERN, TARGET_ID_MAX_LENGTH
#
# Important: Json default values should be avoided in case the default value is changed and an old process relied on the default
#
+_PERMANENT_MEDIA_DIR_NAME = videoag_common.config["PERMANENT_MEDIA_DIR"]
+if _PERMANENT_MEDIA_DIR_NAME.endswith("/"):
+ raise ValueError("PERMANENT_MEDIA_DIR must NOT end with /")
+
+
+# noinspection PyUnresolvedReferences
+def get_permanent_lecture_dir(lecture: "Lecture") -> str:
+ """
+ Returns the db path for the directory of the given lecture. Does NOT end with a /
+ """
+ return (
+ f"{_PERMANENT_MEDIA_DIR_NAME}/"
+ f"course-{lecture.course.id}.{lecture.course.handle}/"
+ f"lecture-{lecture.id}.{lecture.time.strftime("%y%m%d")}"
+ )
+
class MediaProcess:
diff --git a/src/videoag_common/miscellaneous/json.py b/src/videoag_common/miscellaneous/json.py
index 8895662..23f66fc 100644
--- a/src/videoag_common/miscellaneous/json.py
+++ b/src/videoag_common/miscellaneous/json.py
@@ -186,7 +186,7 @@ def replace_json_arguments(data: JsonTypes, argument_data: JsonTypes) -> JsonTyp
if not isinstance(data, str):
raise TypeError(f"Unknown json type: {type(data)}")
- def _sub_match(match):
+ def _get_match_value(match, force_string: bool = True):
path = match.group(1)
val = argument_data
for path_ele in path.split("."):
@@ -194,7 +194,16 @@ def replace_json_arguments(data: JsonTypes, argument_data: JsonTypes) -> JsonTyp
raise ValueError(f"Can't replace with path '{path}' because element '{path_ele}' is not present "
f"(or parent has bad type) in argument data")
val = val[path_ele]
+ if force_string:
+ if isinstance(val, str) or isinstance(val, int):
+ val = str(val)
+ else:
+ raise ValueError(f"Can't replace with path '{path}' because result is not a str or int")
return val
- return _JSON_ARGUMENT_PATTERN.sub(_sub_match, data)
+ full_match = _JSON_ARGUMENT_PATTERN.fullmatch(data)
+ if full_match is not None:
+ return _get_match_value(full_match, force_string=False)
+
+ return _JSON_ARGUMENT_PATTERN.sub(_get_match_value, data)
diff --git a/src/videoag_common/objects/course.py b/src/videoag_common/objects/course.py
index 0117ae3..09b7fb0 100644
--- a/src/videoag_common/objects/course.py
+++ b/src/videoag_common/objects/course.py
@@ -172,7 +172,7 @@ class Lecture(DeletableApiObject, VisibilityApiObject, ApiViewPermissionsObject,
from .medium import TargetMedium
return sql.and_(
TargetMedium.lecture_id == Lecture.id,
- TargetMedium.is_active(from_lecture=True)
+ TargetMedium.is_active(usable_check=False, from_lecture=True)
)
@staticmethod
diff --git a/src/videoag_common/objects/medium.py b/src/videoag_common/objects/medium.py
index 497bdd0..9e1160f 100644
--- a/src/videoag_common/objects/medium.py
+++ b/src/videoag_common/objects/medium.py
@@ -1,6 +1,8 @@
from datetime import datetime
from enum import Enum
+from pathlib import Path
+import videoag_common
from videoag_common.database import *
from videoag_common.miscellaneous import *
from videoag_common.api_object import *
@@ -16,6 +18,11 @@ LOAD_TARGET_MEDIUM_PUBLISH_MEDIUM = RelationshipLoadMarker()
LOAD_TARGET_MEDIUM_LECTURE = RelationshipLoadMarker()
+_API_BASE_URL = videoag_common.config["API_BASE_URL"]
+if _API_BASE_URL.endswith("/"):
+ raise ValueError("API_BASE_URL must NOT end with /")
+
+
# A file which was uploaded and needs to be assigned to a lecture, to be converted to a SourceFileTargetMedium
class SourceMedium(DeletableApiObject, Base):
__table_args__ = (
@@ -116,6 +123,42 @@ class TargetMedium(DeletableApiObject, Base):
"polymorphic_on": "type",
"with_polymorphic": "*" # Always load all attributes for all types
}
+ # This isn't pretty. With a joined table inheritance it might be a bit nicer, but we couldn't check is_produced
+ # Also with a joined table inheritance we would have much more columns (since here we can reuse columns for different types)
+ __table_args__ = (
+ CheckConstraint(
+ f"type NOT IN ('plain_video', 'plain_audio') OR NOT is_produced OR duration_sec IS NOT NULL",
+ name="check_duration_sec_not_null"
+ ),
+ CheckConstraint(
+ f"type NOT IN ('plain_video', 'plain_audio', 'thumbnail') OR NOT is_produced OR file_path IS NOT NULL",
+ name="check_file_path_not_null"
+ ),
+ CheckConstraint(
+ f"type NOT IN ('plain_video', 'plain_audio') OR NOT is_produced OR audio_sample_rate IS NOT NULL",
+ name="check_audio_sample_rate_not_null"
+ ),
+ CheckConstraint(
+ f"type NOT IN ('plain_video', 'plain_audio') OR NOT is_produced OR audio_channel_count IS NOT NULL",
+ name="check_audio_channel_count_not_null"
+ ),
+ CheckConstraint(
+ f"type NOT IN ('plain_video') OR NOT is_produced OR video_vertical_resolution IS NOT NULL",
+ name="check_video_vertical_resolution_not_null"
+ ),
+ CheckConstraint(
+ f"type NOT IN ('plain_video') OR NOT is_produced OR video_horizontal_resolution IS NOT NULL",
+ name="check_video_horizontal_resolution_not_null"
+ ),
+ CheckConstraint(
+ f"type NOT IN ('plain_video') OR NOT is_produced OR video_frame_rate_numerator IS NOT NULL",
+ name="check_video_frame_rate_numerator_not_null"
+ ),
+ CheckConstraint(
+ f"type NOT IN ('plain_video') OR NOT is_produced OR video_frame_rate_denominator IS NOT NULL",
+ name="check_video_frame_rate_denominator_not_null"
+ ),
+ )
# Note that these four may NOT be unique. E.g. if one output is generated twice (because another output of the same
# producer was deleted, etc.)
@@ -210,13 +253,11 @@ class FileMedium(ApiObject):
data_notes="URL to the mediums's file (Maybe with a redirect)"
)
def url(self) -> str:
- # This is not too nice and only works from the API, but we need to access the API config
- import api
- return f"{api.config['API_BASE_URL']}/resources/target_medium/{self.id}"
+ return f"{_API_BASE_URL}/resources/target_medium/{self.id}"
def get_default_file_path_no_ending(self):
- # TODO path
- return f"{self.process_target_id}.{self.id}"
+ assert isinstance(self, TargetMedium)
+ return f"{get_permanent_lecture_dir(self.lecture)}/target-{self.id}.{self.process_target_id}"
class SingleAudioContainingMedium(ApiObject):
--
GitLab