[MINOR] Fix possible 404 errors + get rid of unnecessary arguments (#18)

* fix

* actual fix + simplify code

* update README.md

* [PATCH] Manually trigger version bump

* fix lint

* [PATCH] Sync tests with updated test repo & manually trigger version bump

* fix test_objects.py

Author: ValentinFrancois

README.md

@@ -122,9 +122,7 @@ iterate_subprojects(
     gl: Union[Gitlab, ProjectManager],
     ref: Optional[str] = None,
     only_gitlab_subprojects: bool = False,
-    self_managed_gitlab_host: Optional[str] = None,
-    get_latest_commit_possible_if_not_found: bool = False,
-    get_latest_commit_possible_ref: Optional[str] = None
+    self_managed_gitlab_host: Optional[str] = None
 ) -> Generator[Subproject, None, None]
 ```
 Parameters:
@@ -140,17 +138,6 @@ Parameters:
 - `self_managed_gitlab_host`: (optional) if some submodules are hosted on a 
   self-managed GitLab instance, you should pass its url here otherwise it 
   may be impossible to know from the URL that it's a GitLab project.
-- `get_latest_commit_possible_if_not_found`: (optional) in some rare cases, 
-  there won't be any `Subproject commit ...` info in the diff of the last 
-  commit that updated the submodules. Set this option to `True` if you want to 
-  get instead the most recent commit in the subproject that is anterior to the 
-  commit that updated the submodules of the project. If your goal is to 
-  check that your submodules are up-to-date, you might want to use this.
-- `get_latest_commit_possible_ref`: (optional) in case you set 
-  `get_latest_commit_possible_if_not_found` to `True`, you can specify a ref for the 
-  subproject (for instance your submodule could point to a different branch 
-  than the main one). By default, the main branch of the subproject will be 
-  used.
 
 Returns: Generator of `Subproject` objects
 
@@ -169,8 +156,6 @@ Attributes:
 - `commit: Union[gitlab.v4.objects.ProjectCommit, Commit]`: the commit that 
   the submodule points to (if the submodule is not hosted on GitLab, it will 
   be a dummy `Commit` object with a single attribute `id`)
-- `commit_is_exact: bool`: `True` most of the time, `False` only if the commit 
-  had to be guessed via the `get_latest_commit_possible_if_not_found` option
 
 Example `str()` output:
 ```
@@ -216,16 +201,19 @@ Example `str()` output:
 Converts a `Submodule` object to a [`Subproject`](#class-subproject) object, assuming it's 
 hosted on Gitlab.
 
+Raises as `FileNotFoundError` if the path of the submodule actually doesn't 
+exist in the host repo or if the url of the submodule doesn't link to an 
+existing repo (both can happen if you modify the `.gitmodules` file without 
+using one of the `git submodule` commands)
+
 ```python
 submodule_to_subproject(
     gitmodules_submodule: Submodule,
     gl: Union[Gitlab, ProjectManager],
     self_managed_gitlab_host: Optional[str] = None,
-    get_latest_commit_possible_if_not_found: bool = False,
-    get_latest_commit_possible_ref: Optional[str] = None
 ) -> Subproject
 ```
-Parameters: See [`iterate_subprojects(...)`](#iterate_subprojects)
+Parameter details: See [`iterate_subprojects(...)`](#iterate_subprojects)
 
 
 ## Contributing

gitlab_submodule/gitlab_submodule.py

@@ -23,48 +23,45 @@ def _get_project_manager(
 def submodule_to_subproject(
         gitmodules_submodule: Submodule,
         gl: Union[Gitlab, ProjectManager],
-        self_managed_gitlab_host: Optional[str] = None,
-        get_latest_commit_possible_if_not_found: bool = False,
-        get_latest_commit_possible_ref: Optional[str] = None
+        self_managed_gitlab_host: Optional[str] = None
 ) -> Subproject:
-    submodule_project = submodule_to_project(
-        gitmodules_submodule,
-        _get_project_manager(gl),
-        self_managed_gitlab_host
-    )
-    submodule_commit, commit_is_exact = get_submodule_commit(
-        gitmodules_submodule,
-        submodule_project,
-        get_latest_commit_possible_if_not_found,
-        get_latest_commit_possible_ref
-    )
-    return Subproject(
-        gitmodules_submodule,
-        submodule_project,
-        submodule_commit,
-        commit_is_exact
-    )
+    try:
+        submodule_project = submodule_to_project(
+            gitmodules_submodule,
+            _get_project_manager(gl),
+            self_managed_gitlab_host
+        )
+        submodule_commit = get_submodule_commit(
+            gitmodules_submodule,
+            submodule_project,
+        )
+        return Subproject(
+            gitmodules_submodule,
+            submodule_project,
+            submodule_commit,
+        )
+    except FileNotFoundError:
+        raise
 
 
 def iterate_subprojects(
         project: Project,
         gl: Union[Gitlab, ProjectManager],
         ref: Optional[str] = None,
         only_gitlab_subprojects: bool = False,
-        self_managed_gitlab_host: Optional[str] = None,
-        get_latest_commit_possible_if_not_found: bool = False,
-        get_latest_commit_possible_ref: Optional[str] = None
+        self_managed_gitlab_host: Optional[str] = None
 ) -> Generator[Subproject, None, None]:
     for gitmodules_submodule in iterate_submodules(project, ref):
-        subproject: Subproject = submodule_to_subproject(
-            gitmodules_submodule,
-            _get_project_manager(gl),
-            self_managed_gitlab_host,
-            get_latest_commit_possible_if_not_found,
-            get_latest_commit_possible_ref
-        )
-        if not (only_gitlab_subprojects and not subproject.project):
-            yield subproject
+        try:
+            subproject: Subproject = submodule_to_subproject(
+                gitmodules_submodule,
+                _get_project_manager(gl),
+                self_managed_gitlab_host,
+            )
+            if not (only_gitlab_subprojects and not subproject.project):
+                yield subproject
+        except FileNotFoundError:
+            pass
 
 
 def list_subprojects(*args, **kwargs) -> List[Subproject]:

gitlab_submodule/objects.py

@@ -61,12 +61,10 @@ class Subproject:
     def __init__(self,
                  submodule: Submodule,
                  project: Optional[Project],
-                 commit: Union[ProjectCommit, Commit],
-                 commit_is_exact: bool):
+                 commit: Union[ProjectCommit, Commit]):
         self.submodule = submodule
         self.project = project
         self.commit = commit
-        self.commit_is_exact = commit_is_exact
 
     def __getattribute__(self, item: str):
         try:

gitlab_submodule/submodule_commit.py

@@ -1,41 +1,34 @@
-from typing import Optional, Tuple, Union
+from typing import Optional, Union
 
 import re
 
 from gitlab.v4.objects import Project, ProjectCommit
+from gitlab.exceptions import GitlabGetError
 
 from gitlab_submodule.objects import Submodule, Commit
 
 
 def get_submodule_commit(
         submodule: Submodule,
         submodule_project: Optional[Project] = None,
-        *args,
-        **kwargs
- ) -> Tuple[Union[ProjectCommit, Commit], bool]:
-    commit_id, is_exact = _get_submodule_commit_id(
+ ) -> Union[ProjectCommit, Commit]:
+    commit_id = _get_submodule_commit_id(
         submodule.parent_project,
         submodule.path,
         submodule.parent_ref,
-        submodule_project,
-        *args,
-        **kwargs
     )
     if submodule_project is not None:
         commit = submodule_project.commits.get(commit_id)
     else:
         commit = Commit(commit_id)
-    return commit, is_exact
+    return commit
 
 
 def _get_submodule_commit_id(
     project: Project,
     submodule_path: str,
     ref: Optional[str] = None,
-    submodule_project: Optional[Project] = None,
-    get_latest_commit_possible_if_not_found: bool = False,
-    get_latest_commit_possible_ref: Optional[str] = None
-) -> Tuple[str, bool]:
+) -> str:
     """This uses a trick:
     - The .gitmodules files doesn't contain the actual commit sha that the
       submodules points to.
@@ -46,16 +39,17 @@ def _get_submodule_commit_id(
     => We use that info to get the diff of the last commit that updated the
        submodule commit
     => We parse the diff to get the new submodule commit sha
-
-    NOTE: in some weird cases I observed without really understanding,
-    a commit which created a .gitmodules file can contain zero submodule
-    commit sha in its entire diff.
-    In that case, we can only try to guess which was the latest commit in
-    the submodule project at the datetime of the commit.
     """
-    submodule_dir = project.files.get(
-        submodule_path,
-        ref=ref if ref else project.default_branch)
+    try:
+        submodule_dir = project.files.get(
+            submodule_path,
+            ref=ref if ref else project.default_branch)
+    except GitlabGetError:
+        raise FileNotFoundError(
+           f'Local submodule path "{submodule_path}" was not found for '
+           f'project at url "{project.web_url}" - check if your .gitmodules '
+           f'file is up-to-date.')
+
     last_commit_id = submodule_dir.last_commit_id
     update_submodule_commit = project.commits.get(last_commit_id)
 
@@ -68,26 +62,11 @@ def _get_submodule_commit_id(
             matches = re.findall(submodule_commit_regex, diff_file['diff'])
             # submodule commit id was updated
             if len(matches) == 2:
-                return matches[1], True
+                return matches[1]
             # submodule was added
             if len(matches) == 1:
-                return matches[0], True
-
-    # If the commit diff doesn't contain the submodule commit info, we still
-    # know the date of the last commit in the project that updated the
-    # submodule, so we can fallback to the last commit in the submodule that
-    # was created before this date.
-    # This requires a Project object for the submodule so if it wasn't
-    # passed we cannot guess anything.
-    if not (get_latest_commit_possible_if_not_found
-            and submodule_project is not None):
-        raise ValueError(
-            f'Could not find commit id for submodule {submodule_path} of '
-            f'project {project.path_with_namespace}.')
+                return matches[0]
 
-    last_subproject_commits = submodule_project.commits.list(
-        ref_name=(get_latest_commit_possible_ref
-                  if get_latest_commit_possible_ref
-                  else submodule_project.default_branch),
-        until=update_submodule_commit.created_at)
-    return last_subproject_commits[0].id, False
+    # should never happen
+    raise RuntimeError(f'Did not find any commit id for submodule '
+                       f'"{submodule_path}" at url "{project.web_url}"')

gitlab_submodule/submodule_to_project.py

@@ -5,6 +5,7 @@
 from giturlparse import parse, GitUrlParsed
 
 from gitlab.v4.objects import Project, ProjectManager
+from gitlab.exceptions import GitlabGetError
 
 from gitlab_submodule.objects import Submodule
 from gitlab_submodule.string_utils import lstrip, rstrip
@@ -22,8 +23,15 @@ def submodule_to_project(
                                               self_managed_gitlab_host)
     if not submodule_project_path_with_namespace:
         return None
-    submodule_project = project_manager.get(
-        submodule_project_path_with_namespace)
+    try:
+        submodule_project = project_manager.get(
+            submodule_project_path_with_namespace)
+    except GitlabGetError:
+        # Repo doesn't actually exist (possible because you can modify
+        # .gitmodules without using `git submodule add`)
+        raise FileNotFoundError(
+            'No repo found at url "{}" for submodule at path "{}" - Check if '
+            'the repo was deleted.'.format(submodule.url, submodule.path))
     return submodule_project
 
 

tests/test_gitmodules_to_project.py

@@ -33,15 +33,20 @@ def test_get_submodules_as_projects_with_gitlab_relative_urls(self):
         project = self.gl.projects.get(
             'python-gitlab-submodule-test/test-projects/gitlab-relative-urls')
         submodules = list_project_submodules(project, ref='main')
-        submodule_projects = [
+
+        existing_submodule_projects = [
             submodule_to_project(submodule, self.gl.projects)
-            for submodule in submodules]
+            for submodule in submodules[:4]]
         self.assertTrue(all(
             isinstance(project, Project)
-            for project in submodule_projects))
+            for project in existing_submodule_projects))
         self.assertEqual(
             {'1', '2', '3', '4'},
-            {project.name for project in submodule_projects})
+            {project.name for project in existing_submodule_projects})
+
+        for submodule in submodules[4:]:
+            with self.assertRaises(FileNotFoundError):
+                submodule_to_project(submodule, self.gl.projects)
 
     def test_get_submodules_as_projects_with_external_urls(self):
         project = self.gl.projects.get(

tests/test_objects.py

@@ -98,8 +98,7 @@ def test_Subproject_get_attr(self):
         mock_commit = DictMock()
         mock_commit.id = '123456789'
 
-        project_submodule = Subproject(
-            submodule, mock_project, mock_commit, commit_is_exact=True)
+        project_submodule = Subproject(submodule, mock_project, mock_commit)
 
         self.assertEqual(project_submodule.project, mock_project)
         self.assertEqual(project_submodule.project.name, 'project')
@@ -130,8 +129,7 @@ def test_Subproject_set_attr(self):
         mock_commit = DictMock()
         mock_commit.id = '123456789'
 
-        project_submodule = Subproject(
-            submodule, mock_project, mock_commit, commit_is_exact=True)
+        project_submodule = Subproject(submodule, mock_project, mock_commit)
 
         project_submodule.project_name = 'project2'
         self.assertEqual(project_submodule.project_name, 'project2')
@@ -168,8 +166,7 @@ def test_Subproject_str(self):
         mock_commit = DictMock()
         mock_commit.id = '123456789'
 
-        project_submodule = Subproject(
-            submodule, mock_project, mock_commit, commit_is_exact=True)
+        project_submodule = Subproject(submodule, mock_project, mock_commit)
 
         str_lines = str(project_submodule).split('\n')
         self.assertEqual(
@@ -191,8 +188,7 @@ def test_Subproject_str(self):
             str_lines[2]
         )
         self.assertEqual(
-            "    'commit': <class 'DictMock'> => {'id': '123456789', "
-            "'is_exact': True}",
+            "    'commit': <class 'DictMock'> => {'id': '123456789'}",
             str_lines[3]
         )
         self.assertEqual('}', str_lines[4])
@@ -214,8 +210,7 @@ def test_Subproject_repr(self):
         mock_commit = DictMock()
         mock_commit.id = '123456789'
 
-        project_submodule = Subproject(
-            submodule, mock_project, mock_commit, commit_is_exact=True)
+        project_submodule = Subproject(submodule, mock_project, mock_commit)
 
         str_lines = repr(project_submodule).split('\n')
         self.assertEqual(
@@ -234,7 +229,7 @@ def test_Subproject_repr(self):
             str_lines[2]
         )
         self.assertEqual(
-            "    {'id': '123456789', 'is_exact': True}",
+            "    {'id': '123456789'}",
             str_lines[3]
         )
         self.assertEqual(')', str_lines[4])

tests/test_read_gitmodules.py

@@ -31,7 +31,8 @@ def test_gitmodules_with_gitlab_relative_urls(self):
             {'../../dummy-projects/1.git',
              '../../../python-gitlab-submodule-test/dummy-projects/2.git',
              './../../../python-gitlab-submodule-test/dummy-projects/3.git',
-             './../../dummy-projects/4.git'},
+             './../../dummy-projects/4.git',
+             './../../missing-repos/5.git'},
             {submodule.url for submodule in submodules})
 
     def test_gitmodules_with_external_urls(self):