-
Notifications
You must be signed in to change notification settings - Fork 16
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
Specify tagger for register
and promote
#174
Conversation
Now it looks a bit strange. We could rename it to |
register
and promote
In other words |
Fixed, @Suor. Please check out you have everything you need and I'm merging this. |
gto/tag.py
Outdated
GIT_COMMITTER_NAME: str = None, | ||
GIT_COMMITTER_EMAIL: str = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still looks weird to me eye, though I haven't see all the code. What is the corresponding read function? This one is expected to receive the things that one is returning, i.e.
- if this is gto level abstraction it should be
author/auhtor_email
- if this is git level then
tagger...
will be appropriate.
GIT_COMMITTER_NAME
is implementation detail of gitpython, so if this function is a wrapper to abstract that away it should not leak it like this. Imagine you were to replace gitpython with something else then this function will fail to contain the changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see your point, so let it be tagger
then.
Just to note, GIT_COMMITTER_NAME
is the implementation detail of git
and not gitpython
, since it's the env var git
uses to determine who created the git tag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, it's implementation detail of git
cmdline but not git entirely, i.e. it will make no sense for libgit probably.
gto/tag.py
Outdated
message: str, | ||
GIT_COMMITTER_NAME: str = None, | ||
GIT_COMMITTER_EMAIL: str = None, | ||
): | ||
if all(c.hexsha != ref for c in repo.iter_commits()): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not about this PR, but looks dangerous. You are listing all the commits for existence test. Two more efficient approaches:
repo.commit(ref)
and catch a possible error- try
repo.git.tag(...)
and catch an exception (probably less nice than the one from above)
Second should be more efficient, providing a zero overhead for a happy path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Will do.
with environ( | ||
GIT_AUTHOR_NAME=GIT_AUTHOR_NAME, | ||
GIT_AUTHOR_EMAIL=GIT_AUTHOR_EMAIL, | ||
GIT_COMMITTER_NAME=GIT_COMMITTER_NAME, | ||
GIT_COMMITTER_EMAIL=GIT_COMMITTER_EMAIL, | ||
): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For pytest one might use a monkeypatch fixture instead. What is the purpose of this test though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test checks that check-ref
gives the expected output. That the created git tag will be interpreted by GTO as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But why are you using environ()
when? And even passing stuff like GIT_AUTHOR_NAME
that doesn't do anything.
def _assert_equals(a, b): | ||
# separate function is helpful for debug | ||
# cause you see dicts without skip_keys | ||
assert a == b, f"\n{a} \n!=\n {b}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what is this about but won't pytest -vv
show compared dicts in full?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that when dicts are too long, pytest hides some characters, hiding what differs. E.g.
obj = {'artifact': 'nn', 'author': 'Alexander Guschin', 'author_email': '1aguschin@gmail.com', 'commit_hexsha': '7b745d70573b0ac4bbf6b94c82db42d06f9f4f96', ...}
values = {'artifact': 'nn', 'author': 'Alexander Guschin', 'author_email': '1aguschin@gmail.com', 'commit_hexsha': 'd1d973669cade722f2900e75379cee42fe6b0244', ...}
skip_keys = []
pytest -vv
doesn't solve that.
And apart of that, sometimes skip_keys
occupy a lot of space, making it harder to find the difference. So I come up with this workaround. Would be great to know about better options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- As long as you assert after skipping keys pytest won't show them.
-vv
does show dicts fully, i.e.{pprint(a)} {op} {pprint(b}}
, it also shows a side by side diff.
May also take a look at pytest-clarity
plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'll take a look. Doesn't work for me though, not sure why:
(gto) gto (feature/specify-tagger) $ pytest tests/test_registry.py -vv 7.278s
================================================================= test session starts ==================================================================
platform darwin -- Python 3.9.5, pytest-7.1.1, pluggy-1.0.0 -- /Users/aguschin/.local/share/virtualenvs/gto-mP9zWfJO/bin/python
cachedir: .pytest_cache
rootdir: /Users/aguschin/Git/iterative/gto, configfile: setup.cfg
plugins: mock-3.7.0, lazy-fixture-0.6.3, cov-3.0.0
collected 1 item
tests/test_registry.py::test_registry_state_tag_tag FAILED [100%]
======================================================================= FAILURES =======================================================================
_____________________________________________________________ test_registry_state_tag_tag ______________________________________________________________
showcase = ('/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0',...700>, <git.Commit "00c6d6aba655f1c1ce19340ce866dba7657d43ea">, <git.Commit "c22f41248cdd3c2eaeb756990e3197b23452dbb0">)
def test_registry_state_tag_tag(showcase):
path, repo, write_file, first_commit, second_commit = showcase
reg = GitRegistry.from_repo(repo)
state = reg.get_state().dict()
# TODO: update state
exclude = {
"commits": [],
"versions": [
# "author",
# "author_email",
# "created_at",
# "commit_hexsha",
# "promotions",
# "enrichments",
# "message",
],
"promotions": [
"author",
"author_email",
"created_at",
"commit_hexsha",
"message",
],
}
> _check_state(state, EXPECTED_REGISTRY_TAG_TAG_STATE, exclude)
tests/test_registry.py:205:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/test_registry.py:170: in _check_state
_check_dict(appeared, expected, exclude[part])
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
obj = {'artifact': 'nn', 'author': 'Alexander Guschin', 'author_email': '1aguschin@gmail.com', 'commit_hexsha': '00c6d6aba655f1c1ce19340ce866dba7657d43ea', ...}
values = {'artifact': 'nn', 'author': 'Alexander Guschin', 'author_email': '1aguschin@gmail.com', 'commit_hexsha': 'd1d973669cade722f2900e75379cee42fe6b0244', ...}
skip_keys = []
def _check_dict(
obj: Dict[str, Any],
values: Dict[str, Any],
skip_keys: Union[Set[str], Sequence[str]],
):
obj_values = omit(obj, skip_keys)
values = omit(values, skip_keys)
> assert obj_values == values
E AssertionError
tests/utils.py:21: AssertionError
------------------------------------------------------------------ Captured log setup ------------------------------------------------------------------
DEBUG git.cmd:cmd.py:870 Popen(['git', 'init'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=None)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch-check'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch-check'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch-check'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch-check'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'tag', '-a', 'rf@v1.2.3', '-m', 'Registering artifact rf version v1.2.3', '00c6d6aba655f1c1ce19340ce866dba7657d43ea'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=None)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch-check'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'tag', '-a', 'nn@v0.0.1', '-m', 'Registering artifact nn version v0.0.1', '00c6d6aba655f1c1ce19340ce866dba7657d43ea'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=None)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch-check'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'tag', '-a', 'rf@v1.2.4', '-m', 'Registering artifact rf version v1.2.4', 'c22f41248cdd3c2eaeb756990e3197b23452dbb0'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=None)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch-check'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'tag', '-a', 'nn#staging#1', '-m', 'Promoting nn version v0.0.1 to stage staging', '00c6d6aba655f1c1ce19340ce866dba7657d43ea'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=None)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch-check'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'tag', '-a', 'rf#production#1', '-m', 'Promoting rf version v1.2.3 to stage production', '00c6d6aba655f1c1ce19340ce866dba7657d43ea'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=None)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch-check'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'tag', '-a', 'rf#staging#2', '-m', 'Promoting rf version None to stage staging', 'c22f41248cdd3c2eaeb756990e3197b23452dbb0'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=None)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch-check'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'tag', '-a', 'rf#production#3', '-m', 'Promoting rf version None to stage production', 'c22f41248cdd3c2eaeb756990e3197b23452dbb0'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=None)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch-check'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'cat-file', '--batch'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=<valid stream>)
DEBUG git.cmd:cmd.py:870 Popen(['git', 'tag', '-a', 'rf#production#4', '-m', 'Promoting rf version v1.2.3 to stage production', '00c6d6aba655f1c1ce19340ce866dba7657d43ea'], cwd=/private/var/folders/tv/l60j0x050p536g3bh8g2w1n80000gn/T/pytest-of-aguschin/pytest-43/test_registry_state_tag_tag0, universal_newlines=False, shell=None, istream=None)
---------- coverage: platform darwin, python 3.9.5-final-0 -----------
Name Stmts Miss Cover Missing
---------------------------------------------------
gto/__init__.py 7 0 100%
gto/_gto_version.py 2 0 100%
gto/_version.py 11 7 36% 4-11
gto/api.py 100 69 31% 24-28, 33, 37, 65, 129, 139, 150, 157-158, 170, 202-234, 249-292, 299-305, 320-406
gto/base.py 151 33 78% 48, 56-58, 70, 90, 111, 114-116, 147-161, 178, 229, 231, 258, 268, 273, 278, 291-298
gto/cli.py 264 264 0% 1-828
gto/config.py 87 12 86% 30, 41, 61, 66, 73, 78-79, 85-86, 111, 141-142
gto/constants.py 16 0 100%
gto/exceptions.py 107 41 62% 5-7, 18-19, 26-27, 34-35, 42-43, 50-51, 58-59, 66-67, 74-75, 82-83, 90-91, 98-99, 106-107, 114-115, 122-123, 134-135, 142-143, 150-151, 162-163, 170-171
gto/ext.py 39 11 72% 13-19, 30, 104, 110-111
gto/ext_dvc.py 0 0 100%
gto/ext_mlem.py 0 0 100%
gto/index.py 255 63 75% 48, 65, 74, 78-79, 83-85, 130, 138, 140-147, 161-163, 187, 191, 205-207, 215-217, 220, 249-250, 268, 274-276, 279-284, 291-295, 298, 301-302, 310-316, 327-329, 334, 336, 342, 398-399, 407, 410, 428, 436
gto/log.py 7 1 86% 10
gto/registry.py 126 39 69% 42-43, 78, 116, 124, 128, 130, 147, 164, 188, 190, 201, 207-211, 220, 238, 244-248, 255-256, 265, 271-274, 277, 280, 289-298
gto/tag.py 157 49 69% 47, 49, 62, 74-80, 91, 103-108, 119-121, 130-133, 166, 169, 174, 176, 178, 196-197, 199, 203, 205, 229, 237-248, 312-321, 352-361
gto/ui.py 55 22 60% 20-28, 33-34, 39-40, 44-45, 49-56, 60-62, 67, 72
gto/utils.py 49 33 33% 18-20, 26-46, 54-75, 83
gto/versions.py 63 22 65% 13, 21, 42-43, 55, 57, 63-67, 70-74, 77, 80, 87, 89, 91, 95
---------------------------------------------------
TOTAL 1496 666 55%
Coverage XML written to file coverage.xml
================================================================== slowest durations ===================================================================
5.29s setup tests/test_registry.py::test_registry_state_tag_tag
0.05s call tests/test_registry.py::test_registry_state_tag_tag
0.00s teardown tests/test_registry.py::test_registry_state_tag_tag
=============================================================== short test summary info ================================================================
FAILED tests/test_registry.py::test_registry_state_tag_tag - AssertionError
================================================================== 1 failed in 5.78s ===================================================================
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like you disabled pytest explanations, see:
Codecov Report
@@ Coverage Diff @@
## main #174 +/- ##
==========================================
+ Coverage 81.64% 83.26% +1.62%
==========================================
Files 16 16
Lines 1487 1494 +7
==========================================
+ Hits 1214 1244 +30
+ Misses 273 250 -23
Continue to review full report at Codecov.
|
close #171