Skip to content

Commit

Permalink
[#120] Removed the Version.variant_name attribute.
Browse files Browse the repository at this point in the history
  • Loading branch information
eoyilmaz committed Nov 28, 2024
1 parent f97a8e5 commit d661679
Show file tree
Hide file tree
Showing 11 changed files with 63 additions and 277 deletions.
3 changes: 1 addition & 2 deletions src/stalker/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ class Config(ConfigBase):
admin_group_name="admins",
# the default keyword which is going to be used in password scrambling
key="stalker_default_key",
version_variant_name="Main",
actions=["Create", "Read", "Update", "Delete", "List"], # CRUDL
# Tickets
ticket_label="Ticket",
Expand Down Expand Up @@ -301,7 +300,7 @@ class Config(ConfigBase):
}""",
tj_command="tj3" if sys.platform == "win32" else "/usr/local/bin/tj3",
path_template="{{project.code}}/{%- for parent_task in parent_tasks -%}{{parent_task.nice_name}}/{%- endfor -%}", # noqa: B950
filename_template='{{task.entity_type}}_{{task.id}}_{{version.variant_name}}_v{{"%03d"|format(version.version_number)}}', # noqa: B950
filename_template='{{version.nice_name}}_v{{"%03d"|format(version.version_number)}}', # noqa: B950
# --------------------------------------------
# the following settings came from oyProjectManager
sequence_format="%h%p%t %R",
Expand Down
6 changes: 5 additions & 1 deletion src/stalker/models/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -367,9 +367,13 @@ def _validate_resource(self, key: str, resource: User) -> User:
break

if clashing_time_log_data:
import tzlocal

local_tz = tzlocal.get_localzone()
raise OverBookedError(
"The resource has another TimeLog between {} and {}".format(
clashing_time_log_data[0], clashing_time_log_data[1]
clashing_time_log_data[0].astimezone(local_tz),
clashing_time_log_data[1].astimezone(local_tz),
)
)

Expand Down
5 changes: 2 additions & 3 deletions src/stalker/models/variant.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@ class Variant(Task):
between different variants and being able to review them individually.
You see, in previous versions of Stalker, the variants were handled as a
part of the Version instances with a str attribute called `variant_name`.
The down side of that design was not being able to distinguish any reviews
per variant.
part of the Version instances with a str attribute. The down side of that
design was not being able to distinguish any reviews per variant.
So, when a Model task is approved, all its variant approved all together,
even if one of the variants were still getting worked on.
Expand Down
81 changes: 2 additions & 79 deletions src/stalker/models/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,6 @@ class Version(Link, DAGMixin):
$REPO{{project.repository.id}}/{{project.code}}/{%- for parent_task in parent_tasks -%}{{parent_task.nice_name}}/{%- endfor -%}
Args:
variant_name (str): A short string holding the current variant name. Variants in
Stalker are used for creating variants versions. Versions with the same
``variant_name`` (of the same Task) are numbered together. It can be any
alphanumeric value (a-zA-Z0-9_). The default is the string "Main". When
skipped it will use the default value. It cannot start with a number. It
cannot have white spaces.
inputs (List[Link]): A list o :class:`.Link` instances, holding the inputs of
the current version. It could be a texture for a Maya file or an image
sequence for Nuke, or anything those you can think as the input for the
Expand Down Expand Up @@ -104,12 +98,6 @@ class Version(Link, DAGMixin):
back_populates="versions",
)

variant_name: Mapped[Optional[str]] = mapped_column(
String(256),
default=defaults.version_variant_name,
doc="""Variants in Versions are used for representing different variants of the
same version.""",
)

version_number: Mapped[int] = mapped_column(
default=1,
Expand Down Expand Up @@ -149,7 +137,6 @@ class Version(Link, DAGMixin):
def __init__(
self,
task: Optional[Task] = None,
variant_name: str = defaults.version_variant_name,
inputs: Optional[List["Version"]] = None,
outputs: Optional[List["Version"]] = None,
parent: Optional["Version"] = None,
Expand All @@ -163,7 +150,6 @@ def __init__(

DAGMixin.__init__(self, parent=parent)

self.variant_name = variant_name
self.task = task
self.version_number = None
if inputs is None:
Expand Down Expand Up @@ -194,62 +180,6 @@ def __repr__(self) -> str:
)
)

@classmethod
def _format_variant_name(cls, variant_name: str) -> str:
"""Format the given variant_name value.
Args:
variant_name (str): The variant name value to be formatted.
Returns:
str: The formatted variant name value.
"""
# remove unnecessary characters
variant_name = re.sub(r"([^a-zA-Z0-9\s_\-@]+)", r"", variant_name).strip()

# replace empty spaces with underscores
variant_name = re.sub(r"[\s]+", "_", variant_name)

# replace multiple underscores with only one
# variant_name = re.sub(r'([_]+)', r'_', variant_name)

# remove any non allowed characters from the start
variant_name = re.sub(r"^[^a-zA-Z0-9]+", r"", variant_name)

return variant_name

@validates("variant_name")
def _validate_variant_name(self, key: str, variant_name: str) -> str:
"""Validate the given variant_name value.
Args:
key (str): The name of the validated column.
variant_name (str): The variant name value to be validated.
Raises:
TypeError: If the variant name value is not a string.
ValueError: If the variant name value is an empty string.
Returns:
str: The validated variant name value.
"""
if not isinstance(variant_name, str):
raise TypeError(
"{}.variant_name should be a string, not {}: '{}'".format(
self.__class__.__name__,
variant_name.__class__.__name__,
variant_name,
)
)

variant_name = self._format_variant_name(variant_name)

if variant_name == "":
raise ValueError(
f"{self.__class__.__name__}.variant_name cannot be an empty string"
)

return variant_name

@property
def latest_version(self) -> "Version":
Expand All @@ -264,7 +194,6 @@ def latest_version(self) -> "Version":
with DBSession.no_autoflush:
latest_version = (
Version.query.filter(Version.task == self.task)
.filter(Version.variant_name == self.variant_name)
.order_by(Version.version_number.desc())
.first()
)
Expand Down Expand Up @@ -506,7 +435,6 @@ def latest_published_version(self) -> "Version":
"""
return (
Version.query.filter_by(task=self.task)
.filter_by(variant_name=self.variant_name)
.filter_by(is_published=True)
.order_by(Version.version_number.desc())
.first()
Expand Down Expand Up @@ -547,14 +475,12 @@ def __eq__(self, other):
Returns:
bool: True if the other object is equal to this one as an Entity, is a
Version instance, has the same task, same variant_name and same
version_number.
Version instance, has the same task and same version_number.
"""
return (
super(Version, self).__eq__(other)
and isinstance(other, Version)
and self.task == other.task
and self.variant_name == other.variant_name
and self.version_number == other.version_number
)

Expand Down Expand Up @@ -605,11 +531,8 @@ def nice_name(self) -> str:
Returns:
str: The nice name.
"""
naming_parents = self.naming_parents
return self._format_nice_name(
"{}_{}".format(
"_".join(map(lambda x: x.nice_name, naming_parents)), self.variant_name
)
"_".join(map(lambda x: x.nice_name, self.naming_parents))
)

def walk_inputs(self, method: int = 0) -> Generator[None, "Version", None]:
Expand Down
13 changes: 3 additions & 10 deletions tests/db/test_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -3758,7 +3758,7 @@ def test_persistence_of_structure(setup_postgresql_db):
name="Character Asset Template",
description="This is the template for character assets",
path="Assets/{{asset_type.name}}/{{pipeline_step.code}}",
filename="{{asset.name}}_{{version.variant_name}}_{{asset_type.name}}_\
filename="{{asset.name}}_{{asset_type.name}}_\
v{{version.version_number}}",
target_entity_type="Asset",
type=v_type,
Expand Down Expand Up @@ -4691,7 +4691,6 @@ def test_persistence_of_version(setup_postgresql_db):
test_version = Version(
name="version for task modeling",
task=test_task,
variant_name="MAIN",
full_path="M:/Shows/Proj1/Seq1/Shots/SH001/Lighting"
"/Proj1_Seq1_Sh001_MAIN_Lighting_v001.ma",
outputs=[
Expand All @@ -4712,7 +4711,6 @@ def test_persistence_of_version(setup_postgresql_db):
test_version_2 = Version(
name="version for task modeling",
task=test_task,
variant_name="MAIN",
full_path="M:/Shows/Proj1/Seq1/Shots/SH001/Lighting"
"/Proj1_Seq1_Sh001_MAIN_Lighting_v002.ma",
inputs=[test_version],
Expand All @@ -4732,7 +4730,6 @@ def test_persistence_of_version(setup_postgresql_db):
is_published = test_version.is_published
full_path = test_version.full_path
tags = test_version.tags
variant_name = test_version.variant_name
# tickets = test_version.tickets
type_ = test_version.type
updated_by = test_version.updated_by
Expand All @@ -4756,7 +4753,6 @@ def test_persistence_of_version(setup_postgresql_db):
assert test_version_db.is_published == is_published
assert test_version_db.full_path == full_path
assert test_version_db.tags == tags
assert test_version_db.variant_name == variant_name
assert test_version_db.type == type_
assert test_version_db.updated_by == updated_by
assert test_version_db.version_number == version_number
Expand All @@ -4774,7 +4770,6 @@ def test_persistence_of_version(setup_postgresql_db):
test_version_3 = Version(
name="version for task modeling",
task=test_task,
variant_name="MAIN",
full_path="M:/Shows/Proj1/Seq1/Shots/SH001/Lighting"
"/Proj1_Seq1_Sh001_MAIN_Lighting_v003.ma",
)
Expand All @@ -4801,9 +4796,7 @@ def test_persistence_of_version(setup_postgresql_db):

# create a new version append it to version_3.children and then delete
# version_3
test_version_4 = Version(
name="version for task modeling", task=test_task, variant_name="MAIN"
)
test_version_4 = Version(name="version for task modeling", task=test_task)
test_version_3.children.append(test_version_4)
DBSession.save(test_version_4)
assert test_version_3.children == [test_version_4]
Expand Down Expand Up @@ -4840,7 +4833,7 @@ def test_persistence_of_version(setup_postgresql_db):
assert test_version_4_db.parent is None

# create a new version and assign it as a child of version_5
test_version_5 = Version(task=test_task, variant_name="MAIN")
test_version_5 = Version(task=test_task)
DBSession.save(test_version_5)
test_version_4.children = [test_version_5]
DBSession.commit()
Expand Down
6 changes: 3 additions & 3 deletions tests/models/test_filename_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def setup_filename_template_tests():
name="Test Type", code="tt", target_entity_type="FilenameTemplate"
),
"path": "ASSETS/{{asset.code}}/{{task.type.code}}/",
"filename": "{{asset.code}}_{{version.variant_name}}_{{task.type.code}}_"
"filename": "{{asset.code}}_{{task.type.code}}_"
"{{version.version}}_{{user.initials}}",
"output_path": "",
"target_entity_type": "Asset",
Expand Down Expand Up @@ -315,7 +315,7 @@ def test_naming_case(setup_postgresql_db):
{%- endif -%}
{%- endfor -%}
{%- set fx = parent_tasks[-2] -%}
_{{fx.name}}_{{version.variant_name}}_v{{"%02d"|format(version.version_number)}}""",
_{{fx.name}}_v{{"%02d"|format(version.version_number)}}""",
)
DBSession.add(ft)

Expand Down Expand Up @@ -357,7 +357,7 @@ def test_naming_case(setup_postgresql_db):
DBSession.add(v)
DBSession.commit()

assert v.filename == "ep101_s001c001_fxA_Main_v01.ma"
assert v.filename == "ep101_s001c001_fxA_v01.ma"


def test__hash__is_working_as_expected(setup_filename_template_tests):
Expand Down
8 changes: 4 additions & 4 deletions tests/models/test_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -4312,7 +4312,7 @@ def test_path_attr_raises_a_runtime_error_if_no_matching_filename_template_found
target_entity_type="Asset",
path="{{project.code}}/{%- for parent_task in parent_tasks -%}"
"{{parent_task.nice_name}}/{%- endfor -%}",
filename="{{task.nice_name}}_{{version.variant_name}}"
filename="{{task.nice_name}}"
'_v{{"%03d"|format(version.version_number)}}{{extension}}',
)
structure = Structure(name="Movie Project Structure", templates=[ft])
Expand Down Expand Up @@ -4343,7 +4343,7 @@ def test_path_attr_is_the_rendered_vers_of_the_related_filename_template_in_the_
target_entity_type="Task",
path="{{project.code}}/{%- for parent_task in parent_tasks -%}"
"{{parent_task.nice_name}}/{%- endfor -%}",
filename="{{task.nice_name}}_{{version.variant_name}}"
filename="{{task.nice_name}}"
'_v{{"%03d"|format(version.version_number)}}{{extension}}',
)

Expand Down Expand Up @@ -4410,7 +4410,7 @@ def test_absolute_path_attr_raises_a_runtime_error_if_no_matching_filename_templ
target_entity_type="Asset",
path="{{project.code}}/{%- for parent_task in parent_tasks -%}"
"{{parent_task.nice_name}}/{%- endfor -%}",
filename="{{task.nice_name}}_{{version.variant_name}}"
filename="{{task.nice_name}}"
'_v{{"%03d"|format(version.version_number)}}{{extension}}',
)

Expand Down Expand Up @@ -4445,7 +4445,7 @@ def test_absolute_path_attr_is_rendered_version_of_related_filename_template_in_
"{%- for parent_task in parent_tasks -%}"
"{{parent_task.nice_name}}/"
"{%- endfor -%}",
filename="{{task.nice_name}}_{{version.variant_name}}"
filename="{{task.nice_name}}"
'_v{{"%03d"|format(version.version_number)}}{{extension}}',
)

Expand Down
2 changes: 1 addition & 1 deletion tests/models/test_ticket.py
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,7 @@ def test_summary_argument_is_not_a_string(setup_ticket_tests):
"Ticket.project should be an instance of "
"stalker.models.project.Project, not dict: "
"'{'project': <Test Project 1 (Project)>, "
"'links': [<TEST_PROJECT_1_Test_Asset_Modeling_of_Asset_1_Main_v001 "
"'links': [<TEST_PROJECT_1_Test_Asset_Modeling_of_Asset_1_v001 "
"(Version)>], 'summary': ['not a string instance'], 'description': "
"'This is the long description', 'priority': 'TRIVIAL', 'reported_by': "
"<Test User ('testuser1') (User)>}'"
Expand Down
15 changes: 9 additions & 6 deletions tests/models/test_time_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -359,9 +359,10 @@ def test_overbooked_error_1(setup_time_log_db_tests):
with pytest.raises(OverBookedError) as cm:
TimeLog(**kwargs)

local_tz = tzlocal.get_localzone()
assert str(cm.value) == "The resource has another TimeLog between {} and {}".format(
time_log1.start,
time_log1.end,
time_log1.start.astimezone(local_tz),
time_log1.end.astimezone(local_tz),
)


Expand All @@ -388,9 +389,10 @@ def test_overbooked_error_2(setup_time_log_db_tests):
with pytest.raises(OverBookedError) as cm:
TimeLog(**kwargs)

local_tz = tzlocal.get_localzone()
assert str(cm.value) == "The resource has another TimeLog between {} and {}".format(
time_log1.start,
time_log1.end,
time_log1.start.astimezone(local_tz),
time_log1.end.astimezone(local_tz),
)


Expand All @@ -417,9 +419,10 @@ def test_overbooked_error_3(setup_time_log_db_tests):
with pytest.raises(OverBookedError) as cm:
TimeLog(**kwargs)

local_tz = tzlocal.get_localzone()
assert str(cm.value) == "The resource has another TimeLog between {} and {}".format(
time_log1.start,
time_log1.end,
time_log1.start.astimezone(local_tz),
time_log1.end.astimezone(local_tz),
)


Expand Down
Loading

0 comments on commit d661679

Please sign in to comment.