Skip to content

Commit

Permalink
Fix check-ref wrong results (#201)
Browse files Browse the repository at this point in the history
* writing tests that pass 🤔

* fix test

* fix bug
  • Loading branch information
aguschin authored Jul 1, 2022
1 parent 9f125e7 commit dfe6e67
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 21 deletions.
4 changes: 2 additions & 2 deletions gto/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,15 +263,15 @@ def update_artifact(self, artifact: BaseArtifact):
def get_artifacts(self):
return self.artifacts

def find_artifact(self, name: str, create_new=False):
def find_artifact(self, name: str, create_new=False) -> BaseArtifact:
if not name:
raise ValueError("Artifact name is required")
if name not in self.artifacts:
if create_new:
self.artifacts[name] = BaseArtifact(name=name, versions=[])
else:
raise ArtifactNotFound(name)
return self.artifacts.get(name)
return self.artifacts[name]

@property
def unique_stages(self):
Expand Down
15 changes: 7 additions & 8 deletions gto/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,11 @@ def promote( # pylint: disable=too-many-locals
and found_version.stage.stage == stage
):
raise WrongArgs(f"Version is already in stage '{stage}'")
self.stage_manager.promote( # type: ignore
# TODO: getting tag name as a result and using it
# is leaking implementation details in base module
# it's roughly ok to have until we add other implementations
# beside tag-based promotions
tag = self.stage_manager.promote( # type: ignore
name,
stage,
ref=promote_ref,
Expand All @@ -237,12 +241,7 @@ def promote( # pylint: disable=too-many-locals
author=author,
author_email=author_email,
)
promotion = (
self.get_state()
.find_artifact(name)
.find_version_at_commit(promote_ref)
.stage
)
promotion = self.stage_manager.check_ref(tag, self.get_state())[name]
if stdout:
echo(
f"Created git tag '{promotion.tag}' that promotes '{promotion.version}'"
Expand Down Expand Up @@ -279,7 +278,7 @@ def latest(self, name: str, all: bool = False, registered: bool = True):
"""Return latest active version for artifact"""
artifact = self.get_state().find_artifact(name)
if all:
return artifact.sort_versions(registered=registered)
return artifact.get_versions(include_non_explicit=not registered)
return artifact.get_latest_version(registered_only=registered)

def _get_allowed_stages(self):
Expand Down
10 changes: 5 additions & 5 deletions gto/tag.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,15 +342,17 @@ def promote(
simple,
author: Optional[str] = None,
author_email: Optional[str] = None,
):
) -> str:
tag = name_tag(Action.PROMOTE, name, stage=stage, repo=self.repo, simple=simple)
create_tag(
self.repo,
name_tag(Action.PROMOTE, name, stage=stage, repo=self.repo, simple=simple),
tag,
ref=ref,
message=message,
tagger=author,
tagger_email=author_email,
)
return tag

def check_ref(self, ref: str, state: BaseRegistryState):
try:
Expand All @@ -366,7 +368,5 @@ def check_ref(self, ref: str, state: BaseRegistryState):
name: promotion
for name, artifact in state.get_artifacts().items()
for promotion in artifact.stages
if name == art_name
and promotion.commit_hexsha == tag.commit.hexsha
and promotion.created_at == datetime.fromtimestamp(tag.tag.tagged_date)
if name == art_name and promotion.tag == tag.name
}
50 changes: 44 additions & 6 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
import pytest

import gto
from gto.constants import STAGE, VERSION, Action
from gto.exceptions import PathIsUsed, WrongArgs
from gto.tag import find
from gto.versions import SemVer
from tests.utils import _check_obj

Expand Down Expand Up @@ -168,11 +170,11 @@ def environ(**overrides):
os.environ.pop(name, None)


def test_check_ref(repo_with_artifact: Tuple[git.Repo, Callable]):
def test_check_ref_detailed(repo_with_artifact: Tuple[git.Repo, Callable]):
repo, name = repo_with_artifact

NAME = "model"
VERSION = "v1.2.3"
SEMVER = "v1.2.3"
GIT_AUTHOR_NAME = "Alexander Guschin"
GIT_AUTHOR_EMAIL = "[email protected]"
GIT_COMMITTER_NAME = "Oliwav"
Expand All @@ -184,25 +186,61 @@ def test_check_ref(repo_with_artifact: Tuple[git.Repo, Callable]):
GIT_COMMITTER_NAME=GIT_COMMITTER_NAME,
GIT_COMMITTER_EMAIL=GIT_COMMITTER_EMAIL,
):
gto.api.register(repo, name=NAME, ref="HEAD", version=VERSION)
gto.api.register(repo, name=NAME, ref="HEAD", version=SEMVER)

info = gto.api.check_ref(repo, f"{NAME}@{VERSION}")["version"][NAME]
info = gto.api.check_ref(repo, f"{NAME}@{SEMVER}")[VERSION][NAME]
_check_obj(
info,
{
"artifact": NAME,
"name": VERSION,
"name": SEMVER,
"author": GIT_COMMITTER_NAME,
"author_email": GIT_COMMITTER_EMAIL,
"discovered": False,
"tag": f"{NAME}@{VERSION}",
"tag": f"{NAME}@{SEMVER}",
"promotions": [],
"enrichments": [],
},
skip_keys={"commit_hexsha", "created_at", "message"},
)


def test_check_ref_multiple_showcase(showcase):
repo: git.Repo
(
path,
repo,
write_file,
first_commit,
second_commit,
) = showcase

tags = find(repo=repo, action=Action.REGISTER)
for tag in tags:
info = list(gto.api.check_ref(repo, tag.name)[VERSION].values())[0]
assert info.tag == tag.name

tags = find(repo=repo, action=Action.PROMOTE)
for tag in tags:
info = list(gto.api.check_ref(repo, tag.name)[STAGE].values())[0]
assert info.tag == tag.name


def test_check_ref_catch_the_bug(repo_with_artifact: Tuple[git.Repo, Callable]):
repo, name = repo_with_artifact
NAME = "artifact"
gto.api.register(repo, NAME, "HEAD")
promotion1 = gto.api.promote(repo, NAME, "staging", promote_ref="HEAD")
promotion2 = gto.api.promote(repo, NAME, "prod", promote_ref="HEAD")
promotion3 = gto.api.promote(repo, NAME, "dev", promote_ref="HEAD")
for promotion, tag in zip(
[promotion1, promotion2, promotion3],
[f"{NAME}#staging#1", f"{NAME}#prod#2", f"{NAME}#dev#3"],
):
info = gto.api.check_ref(repo, tag)[STAGE][NAME]
assert info.tag == promotion.tag == tag


def test_is_not_gto_repo(empty_git_repo):
repo, _ = empty_git_repo
assert not gto.api._is_gto_repo(repo.working_dir)
Expand Down

0 comments on commit dfe6e67

Please sign in to comment.