Repository: valohai/valohai-cli

Issue:
Commits not recognized in projects with custom yaml paths
When running a non-adhoc execution from the cli in a project with custom path for `valohai.yaml` you get the following error: 
````
😩  Warning: Commit 5cd5042ec25980e1b9f32c2978f3dc8bc2e9ed0e is not known for the project. Have you pushed it?
Aborted!
````

It also seems that when fetching the repository to the project, there is a suffix added to the commit hash. Could this be the problem? 
````
😊  Success! Fetched: main (5cd5042ec25980e1b9f32c2978f3dc8bc2e9ed0e-mysubdir)
😊  Success! 1 new commits were fetched!
````

Running the execution from the UI and as an adhoc job both work fine. 

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/valohai/valohai-cli/pull/243?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=valohai) Report
> Merging [#243](https://codecov.io/gh/valohai/valohai-cli/pull/243?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=valohai) (78e5d4b) into [master](https://codecov.io/gh/valohai/valohai-cli/commit/ee107b77419465cbc5422832e4516b0a53040a7e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=valohai) (ee107b7) will **increase** coverage by `0.17%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master     #243      +/-   ##
==========================================
+ Coverage   86.26%   86.43%   +0.17%     
==========================================
  Files         123      124       +1     
  Lines        4054     4092      +38     
  Branches      665      675      +10     
==========================================
+ Hits         3497     3537      +40     
+ Misses        386      383       -3     
- Partials      171      172       +1     
```


| [Impacted Files](https://codecov.io/gh/valohai/valohai-cli/pull/243?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=valohai) | Coverage Δ | |
|---|---|---|
| [tests/test\_utils\_commits.py](https://codecov.io/gh/valohai/valohai-cli/pull/243/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=valohai#diff-dGVzdHMvdGVzdF91dGlsc19jb21taXRzLnB5) | `100.00% <100.00%> (ø)` | |
| [valohai\_cli/models/project.py](https://codecov.io/gh/valohai/valohai-cli/pull/243/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=valohai#diff-dmFsb2hhaV9jbGkvbW9kZWxzL3Byb2plY3QucHk=) | `82.10% <100.00%> (+1.64%)` | :arrow_up: |
| [valohai\_cli/utils/commits.py](https://codecov.io/gh/valohai/valohai-cli/pull/243/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=valohai#diff-dmFsb2hhaV9jbGkvdXRpbHMvY29tbWl0cy5weQ==) | `80.95% <0.00%> (+4.76%)` | :arrow_up: |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=valohai)


Patch:
diff --git a/valohai_cli/commands/execution/run/frontend_command.py b/valohai_cli/commands/execution/run/frontend_command.py
index e840667..b8c129d 100644
--- a/valohai_cli/commands/execution/run/frontend_command.py
+++ b/valohai_cli/commands/execution/run/frontend_command.py
@@ -79,7 +79,7 @@ def run(
 
     if not commit and project.is_remote:
         # For remote projects, we need to resolve early.
-        commit = project.resolve_commit()['identifier']
+        commit = project.resolve_commits()[0]['identifier']
         info(f'Using remote project {project.name}\'s newest commit {commit}')
 
     # We need to pass commit=None when adhoc=True to `get_config`, but
diff --git a/valohai_cli/models/project.py b/valohai_cli/models/project.py
index 67298bc..6472742 100644
--- a/valohai_cli/models/project.py
+++ b/valohai_cli/models/project.py
@@ -108,24 +108,46 @@ class Project:
             self._commit_list = commits
         return self._commit_list
 
-    def resolve_commit(self, commit_identifier: Optional[str] = None) -> dict:
+    def resolve_commits(self, commit_identifier: Optional[str] = None) -> List[dict]:
         """
-        Resolve a commit identifier to a commit dict.
+        Resolve a commit identifier to a list of matching commit dicts.
 
-        :raises KeyError: if an explicitly named identifier is not found
+        If a blank identifier is requested, resolve to a list with just the latest commit.
+
+        Tries to find prefix partial matches if an exact match is not found.
+
+        Commit dicts are returned in descending commit time order.
+
+        :raises KeyError: if an exact or partial identifier is not found
         :raises IndexError: if there are no commits
         """
         commits = self.load_commit_list()
+
         if commit_identifier:
+            # try to find exact match before sorting...
             by_identifier = {c['identifier']: c for c in commits}
-            return by_identifier[commit_identifier]
-        newest_commit = sorted(
+            exact_commit = by_identifier.get(commit_identifier)
+            if exact_commit:
+                return [exact_commit]
+
+        sorted_commits = sorted(
             (c for c in commits if not c.get('adhoc')),
             key=lambda c: str(c['commit_time']),
             reverse=True,
-        )[0]
+        )
+
+        if commit_identifier:
+            partial_matches = [
+                c for c in sorted_commits
+                if c['identifier'].startswith(commit_identifier)
+            ]
+            if not partial_matches:
+                raise KeyError(commit_identifier)
+            return partial_matches
+
+        newest_commit = sorted_commits[0]
         assert newest_commit['identifier']
-        return newest_commit
+        return [newest_commit]
 
     def load_full_commit(self, identifier: Optional[str] = None) -> dict:
         """
diff --git a/valohai_cli/utils/commits.py b/valohai_cli/utils/commits.py
index a0469e3..9c0e73c 100644
--- a/valohai_cli/utils/commits.py
+++ b/valohai_cli/utils/commits.py
@@ -50,7 +50,7 @@ def resolve_commit(commit_identifier: Optional[str], project: Project) -> str:
         return commit_identifier
 
     try:
-        commit_obj = project.resolve_commit(commit_identifier=commit_identifier)
+        matching_commits = project.resolve_commits(commit_identifier=commit_identifier)
     except KeyError:
         warn(f'Commit {commit_identifier} is not known for the project. Have you pushed it?')
         raise click.Abort()
@@ -58,8 +58,15 @@ def resolve_commit(commit_identifier: Optional[str], project: Project) -> str:
         warn('No commits are known for the project.')
         raise click.Abort()
 
+    commit_obj = matching_commits[0]
     resolved_commit_identifier: str = commit_obj['identifier']
-    if not commit_identifier:
+    if len(matching_commits) > 1:
+        ambiguous_str = ", ".join([c['identifier'] for c in matching_commits[1:]])
+        click.echo(
+            f'Resolved to commit {resolved_commit_identifier}, which is ambiguous with {ambiguous_str}',
+            err=True,
+        )
+    elif not commit_identifier:
         click.echo(f'Resolved to commit {resolved_commit_identifier}', err=True)
 
     return resolved_commit_identifier


Test patch:
diff --git a/tests/commands/deployment/test_create_version.py b/tests/commands/deployment/test_create_version.py
index a90c7a1..7ba4e0c 100644
--- a/tests/commands/deployment/test_create_version.py
+++ b/tests/commands/deployment/test_create_version.py
@@ -12,10 +12,10 @@ from valohai_cli.models.project import Project
 def test_create_version(runner, logged_in_and_linked, monkeypatch, name):
     commit_identifier = 'f' * 16
 
-    def mock_resolve_commit(mock_self, *, commit_identifier):
-        return {'identifier': commit_identifier}
+    def mock_resolve_commits(mock_self, *, commit_identifier):
+        return [{'identifier': commit_identifier}]
 
-    monkeypatch.setattr(Project, 'resolve_commit', mock_resolve_commit)
+    monkeypatch.setattr(Project, 'resolve_commits', mock_resolve_commits)
 
     project = get_project()
     apimock_kwargs = {'deployment_version_name': name} if name else {}
diff --git a/tests/commands/execution/test_run.py b/tests/commands/execution/test_run.py
index cb5eda1..a65c34e 100644
--- a/tests/commands/execution/test_run.py
+++ b/tests/commands/execution/test_run.py
@@ -17,10 +17,10 @@ def run_test_setup(request, logged_in_and_linked, monkeypatch):
 
 @pytest.fixture()
 def patch_git(monkeypatch):
-    def mock_resolve_commit(mock_self, *, commit_identifier):
-        return {'identifier': commit_identifier}
+    def mock_resolve_commits(mock_self, *, commit_identifier):
+        return [{'identifier': commit_identifier}]
 
-    monkeypatch.setattr(Project, 'resolve_commit', mock_resolve_commit)
+    monkeypatch.setattr(Project, 'resolve_commits', mock_resolve_commits)
 
 
 def test_run_requires_step(runner, logged_in_and_linked):
diff --git a/tests/test_utils_commits.py b/tests/test_utils_commits.py
new file mode 100644
index 0000000..4edd45a
--- /dev/null
+++ b/tests/test_utils_commits.py
@@ -0,0 +1,91 @@
+import os
+
+import click.exceptions
+import pytest
+import requests_mock
+
+from tests.fixture_data import PROJECT_DATA
+from valohai_cli.models.project import Project
+from valohai_cli.utils.commits import resolve_commit
+
+
+class TestResolveCommit:
+
+    commit_api_url = 'https://app.valohai.com/api/v0/commits/'
+
+    @pytest.fixture
+    def project(self):
+        return Project(data=PROJECT_DATA, directory=os.getcwd())
+
+    @pytest.fixture
+    def commits(self):
+        return [
+            {
+                'commit_time': '2022-11-04T12:32:02Z',  # the oldest
+                'identifier': 'a9f10d48ed9ece3997b40d2432c64eb08bd11e45',
+            },
+            {
+                'commit_time': '2022-11-04T13:33:03Z',  # has ambiguous extension
+                'identifier': 'b5b50f5d183ebc645460dba1329ee48798f31482-alpha',
+            },
+            {
+                'commit_time': '2022-11-04T14:34:04Z',  # has ambiguous extension
+                'identifier': 'b5b50f5d183ebc645460dba1329ee48798f31482-beta',
+            },
+            {
+                'commit_time': '2022-11-04T15:35:05Z',  # has extension
+                'identifier': '8e8351d22cc14e1eeda8c9a5e81320e19eeed3e4-gamma',
+            },
+            {
+                'commit_time': '2022-11-04T16:36:06Z',  # the latest
+                'identifier': 'c243d40ff7feb63aa7e15e8eda1d8859010ebc53',
+            }
+        ]
+
+    @pytest.mark.parametrize('lookup, expected', [
+        # adhoc commit resolves to itself
+        ['~d1df3bdb3614731c4ba795fa3c322617a3c996d9', '~d1df3bdb3614731c4ba795fa3c322617a3c996d9'],
+        # exact match to extensionless commit identifier
+        ['c243d40ff7feb63aa7e15e8eda1d8859010ebc53', 'c243d40ff7feb63aa7e15e8eda1d8859010ebc53'],
+        # partial match to extensionless commit identifier
+        ['c243d40', 'c243d40ff7feb63aa7e15e8eda1d8859010ebc53'],
+        # exact match to commit identifier with an extension
+        ['b5b50f5d183ebc645460dba1329ee48798f31482-alpha', 'b5b50f5d183ebc645460dba1329ee48798f31482-alpha'],
+    ])
+    def test_works(self, capsys, logged_in, project, commits, lookup, expected):
+        with requests_mock.mock() as m:
+            m.get(self.commit_api_url, json={'results': commits})
+            assert resolve_commit(lookup, project=project) == expected
+        out, err = capsys.readouterr()
+        assert not out
+        assert not err
+
+    def test_work_but_warns_if_empty_lookup(self, capsys, logged_in, project, commits):
+        with requests_mock.mock() as m:
+            m.get(self.commit_api_url, json={'results': commits})
+            resolved = resolve_commit('', project=project)
+        assert resolved == 'c243d40ff7feb63aa7e15e8eda1d8859010ebc53'
+        out, err = capsys.readouterr()
+        assert 'Resolved to commit' in err
+        assert 'ambiguous' not in err
+
+    def test_work_but_warns_if_ambiguous(self, capsys, logged_in, project, commits):
+        with requests_mock.mock() as m:
+            m.get(self.commit_api_url, json={'results': commits})
+            # partial match to the latest matching commit identifier with an extension
+            resolved = resolve_commit('b5b50f5d183ebc645460dba1329ee48798f31482', project=project)
+        assert resolved == 'b5b50f5d183ebc645460dba1329ee48798f31482-beta'
+        out, err = capsys.readouterr()
+        assert 'which is ambiguous with' in err
+
+    def test_fails_if_not_found(self, logged_in, project, commits):
+        with requests_mock.mock() as m:
+            m.get(self.commit_api_url, json={'results': commits})
+            with pytest.raises(click.exceptions.Abort):
+                resolve_commit('94c9d70834f53ce765f3b86bac50f015278d8bd4', project)
+
+    def test_fails_if_no_commits(self, logged_in, project):
+        with requests_mock.mock() as m:
+            m.get(self.commit_api_url, json={'results': []})
+            with pytest.raises(click.exceptions.Abort):
+                resolve_commit('c243d40ff7feb63aa7e15e8eda1d8859010ebc53', project)