fix(manifest-connectors): resolve parsing errors during version comparisons (#38)

Author: aaronsteers

.github/workflows/connector-tests.yml

@@ -72,6 +72,8 @@ jobs:
             cdk_extra: n/a
           - connector: source-shopify
             cdk_extra: n/a
+          - connector: source-chargebee
+            cdk_extra: n/a
           # Currently not passing CI (unrelated)
           # - connector: source-zendesk-support
           #   cdk_extra: n/a

airbyte_cdk/sources/declarative/manifest_declarative_source.py

@@ -5,10 +5,10 @@
 import json
 import logging
 import pkgutil
-import re
 from copy import deepcopy
 from importlib import metadata
-from typing import Any, Dict, Iterator, List, Mapping, Optional, Tuple
+from typing import Any, Dict, Iterator, List, Mapping, Optional
+from packaging.version import Version, InvalidVersion
 
 import yaml
 from airbyte_cdk.models import (
@@ -245,45 +245,54 @@ def _validate_source(self) -> None:
                 "Validation against json schema defined in declarative_component_schema.yaml schema failed"
             ) from e
 
-        cdk_version = metadata.version("airbyte_cdk")
-        cdk_major, cdk_minor, cdk_patch = self._get_version_parts(cdk_version, "airbyte-cdk")
-        manifest_version = self._source_config.get("version")
-        if manifest_version is None:
+        cdk_version_str = metadata.version("airbyte_cdk")
+        cdk_version = self._parse_version(cdk_version_str, "airbyte-cdk")
+        manifest_version_str = self._source_config.get("version")
+        if manifest_version_str is None:
             raise RuntimeError(
                 "Manifest version is not defined in the manifest. This is unexpected since it should be a required field. Please contact support."
             )
-        manifest_major, manifest_minor, manifest_patch = self._get_version_parts(
-            manifest_version, "manifest"
-        )
+        manifest_version = self._parse_version(manifest_version_str, "manifest")
 
-        if cdk_version.startswith("0.0.0"):
+        if (cdk_version.major, cdk_version.minor, cdk_version.micro) == (0, 0, 0):
             # Skipping version compatibility check on unreleased dev branch
             pass
-        elif cdk_major < manifest_major or (
-            cdk_major == manifest_major and cdk_minor < manifest_minor
+        elif (cdk_version.major, cdk_version.minor) < (
+            manifest_version.major,
+            manifest_version.minor,
         ):
             raise ValidationError(
-                f"The manifest version {manifest_version} is greater than the airbyte-cdk package version ({cdk_version}). Your "
+                f"The manifest version {manifest_version!s} is greater than the airbyte-cdk package version ({cdk_version!s}). Your "
                 f"manifest may contain features that are not in the current CDK version."
             )
-        elif manifest_major == 0 and manifest_minor < 29:
+        elif (manifest_version.major, manifest_version.minor) < (0, 29):
             raise ValidationError(
                 f"The low-code framework was promoted to Beta in airbyte-cdk version 0.29.0 and contains many breaking changes to the "
-                f"language. The manifest version {manifest_version} is incompatible with the airbyte-cdk package version "
-                f"{cdk_version} which contains these breaking changes."
+                f"language. The manifest version {manifest_version!s} is incompatible with the airbyte-cdk package version "
+                f"{cdk_version!s} which contains these breaking changes."
             )
 
     @staticmethod
-    def _get_version_parts(version: str, version_type: str) -> Tuple[int, int, int]:
-        """
-        Takes a semantic version represented as a string and splits it into a tuple of its major, minor, and patch versions.
+    def _parse_version(
+        version: str,
+        version_type: str,
+    ) -> Version:
+        """Takes a semantic version represented as a string and splits it into a tuple.
+
+        The fourth part (prerelease) is not returned in the tuple.
+
+        Returns:
+            Version: the parsed version object
         """
-        version_parts = re.split(r"\.", version)
-        if len(version_parts) != 3 or not all([part.isdigit() for part in version_parts]):
+        try:
+            parsed_version = Version(version)
+        except InvalidVersion as ex:
             raise ValidationError(
-                f"The {version_type} version {version} specified is not a valid version format (ex. 1.2.3)"
-            )
-        return tuple(int(part) for part in version_parts)  # type: ignore # We already verified there were 3 parts and they are all digits
+                f"The {version_type} version '{version}' is not a valid version format."
+            ) from ex
+        else:
+            # No exception
+            return parsed_version
 
     def _stream_configs(self, manifest: Mapping[str, Any]) -> List[Dict[str, Any]]:
         # This has a warning flag for static, but after we finish part 4 we'll replace manifest with self._source_config

unit_tests/sources/declarative/test_manifest_declarative_source.py

@@ -644,7 +644,22 @@ def test_source_with_missing_version_fails(self):
                 id="manifest_version_has_invalid_minor_format",
             ),
             pytest.param(
-                "0.29.0", "0.29.0.1", ValidationError, id="manifest_version_has_extra_version_parts"
+                "0.29.0",
+                "0.29.0rc1",
+                None,
+                id="manifest_version_is_release_candidate",
+            ),
+            pytest.param(
+                "0.29.0rc1",
+                "0.29.0",
+                None,
+                id="cdk_version_is_release_candidate",
+            ),
+            pytest.param(
+                "0.29.0",
+                "0.29.0.0.3",  # packaging library does not complain and the parts are ignored during comparisons.
+                None,
+                id="manifest_version_has_extra_version_parts",
             ),
             pytest.param(
                 "0.29.0", "5.0", ValidationError, id="manifest_version_has_too_few_version_parts"
@@ -655,7 +670,13 @@ def test_source_with_missing_version_fails(self):
         ],
     )
     @patch("importlib.metadata.version")
-    def test_manifest_versions(self, version, cdk_version, manifest_version, expected_error):
+    def test_manifest_versions(
+        self,
+        version,
+        cdk_version,
+        manifest_version,
+        expected_error,
+    ) -> None:
         # Used to mock the metadata.version() for test scenarios which normally returns the actual version of the airbyte-cdk package
         version.return_value = cdk_version