Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix check-ref wrong results #201

Merged
merged 3 commits into from
Jul 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 = "aguschin@iterative.ai"
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