Make QA checks tolerate connector without dockerfile

airbyte-ci/connectors/connector_ops/connector_ops/qa_checks.py

@@ -5,7 +5,7 @@
 
 import sys
 from pathlib import Path
-from typing import Iterable, Optional, Set, Tuple
+from typing import Callable, Iterable, Optional, Set, Tuple
 
 from connector_ops.utils import Connector, ConnectorLanguage
 from pydash.objects import get
@@ -14,7 +14,7 @@
 def check_migration_guide(connector: Connector) -> bool:
     """Check if a migration guide is available for the connector if a breaking change was introduced."""
 
-    breaking_changes = get(connector.metadata, f"releases.breakingChanges")
+    breaking_changes = get(connector.metadata, "releases.breakingChanges")
     if not breaking_changes:
         return True
 
@@ -27,7 +27,7 @@ def check_migration_guide(connector: Connector) -> bool:
 
     # Check that the migration guide begins with # {connector name} Migration Guide
     expected_title = f"# {connector.name_from_metadata} Migration Guide"
-    expected_version_header_start = f"## Upgrading to "
+    expected_version_header_start = "## Upgrading to "
     with open(migration_guide_file_path) as f:
         first_line = f.readline().strip()
         if not first_line == expected_title:
@@ -55,7 +55,7 @@ def check_migration_guide(connector: Connector) -> bool:
 
         if ordered_breaking_changes != ordered_heading_versions:
             print(f"Migration guide file for {connector.name} has incorrect version headings.")
-            print(f"Check for missing, extra, or misordered headings, or headers with typos.")
+            print("Check for missing, extra, or misordered headings, or headers with typos.")
             print(f"Expected headings: {ordered_expected_headings}")
             return False
 
@@ -242,7 +242,7 @@ def check_metadata_version_matches_dockerfile_label(connector: Connector) -> boo
     return version_in_dockerfile == connector.version
 
 
-QA_CHECKS = [
+DEFAULT_QA_CHECKS = (
     check_documentation_file_exists,
     check_migration_guide,
     # Disabling the following check because it's likely to not pass on a lot of connectors.
@@ -254,8 +254,13 @@ def check_metadata_version_matches_dockerfile_label(connector: Connector) -> boo
     # https://github.com/airbytehq/airbyte/issues/21606
     check_connector_https_url_only,
     check_connector_has_no_critical_vulnerabilities,
-    check_metadata_version_matches_dockerfile_label,
-]
+)
+
+
+def get_qa_checks_to_run(connector: Connector) -> Tuple[Callable]:
+    if connector.has_dockerfile:
+        return DEFAULT_QA_CHECKS + (check_metadata_version_matches_dockerfile_label,)
+    return DEFAULT_QA_CHECKS
 
 
 def remove_strict_encrypt_suffix(connector_technical_name: str) -> str:
@@ -289,7 +294,7 @@ def run_qa_checks():
     connector_technical_name = remove_strict_encrypt_suffix(connector_technical_name)
     connector = Connector(connector_technical_name)
     print(f"Running QA checks for {connector_technical_name}:{connector.version}")
-    qa_check_results = {qa_check.__name__: qa_check(connector) for qa_check in QA_CHECKS}
+    qa_check_results = {qa_check.__name__: qa_check(connector) for qa_check in get_qa_checks_to_run(connector)}
     if not all(qa_check_results.values()):
         print(f"QA checks failed for {connector_technical_name}:{connector.version}:")
         for check_name, check_result in qa_check_results.items():

airbyte-ci/connectors/connector_ops/connector_ops/utils.py

@@ -299,6 +299,10 @@ def icon_path(self) -> Path:
     def code_directory(self) -> Path:
         return Path(f"./airbyte-integrations/connectors/{self.technical_name}")
 
+    @property
+    def has_dockerfile(self) -> bool:
+        return (self.code_directory / "Dockerfile").is_file()
+
     @property
     def metadata_file_path(self) -> Path:
         return self.code_directory / METADATA_FILE_NAME
@@ -321,20 +325,19 @@ def language(self) -> ConnectorLanguage:
         return None
 
     @property
-    def version(self) -> str:
+    def version(self) -> Optional[str]:
         if self.metadata is None:
             return self.version_in_dockerfile_label
         return self.metadata["dockerImageTag"]
 
     @property
     def version_in_dockerfile_label(self) -> Optional[str]:
-        try:
-            with open(self.code_directory / "Dockerfile") as f:
-                for line in f:
-                    if "io.airbyte.version" in line:
-                        return line.split("=")[1].strip()
-        except FileNotFoundError:
+        if not self.has_dockerfile:
             return None
+        with open(self.code_directory / "Dockerfile") as f:
+            for line in f:
+                if "io.airbyte.version" in line:
+                    return line.split("=")[1].strip()
         raise ConnectorVersionNotFound(
             """
             Could not find the connector version from its Dockerfile.

airbyte-ci/connectors/connector_ops/tests/test_qa_checks.py

@@ -80,7 +80,7 @@ def test_run_qa_checks_success(capsys, mocker, user_input, expect_qa_checks_to_r
     mocker.patch.object(qa_checks, "Connector")
     mock_qa_check = mocker.Mock(return_value=True, __name__="mock_qa_check")
     if expect_qa_checks_to_run:
-        mocker.patch.object(qa_checks, "QA_CHECKS", [mock_qa_check])
+        mocker.patch.object(qa_checks, "get_qa_checks_to_run", return_value=[mock_qa_check])
     with pytest.raises(SystemExit) as wrapped_error:
         qa_checks.run_qa_checks()
     assert wrapped_error.value.code == 0
@@ -101,7 +101,7 @@ def test_run_qa_checks_error(capsys, mocker):
     mocker.patch.object(qa_checks.sys, "argv", ["", "source-faker"])
     mocker.patch.object(qa_checks, "Connector")
     mock_qa_check = mocker.Mock(return_value=False, __name__="mock_qa_check")
-    mocker.patch.object(qa_checks, "QA_CHECKS", [mock_qa_check])
+    mocker.patch.object(qa_checks, "DEFAULT_QA_CHECKS", (mock_qa_check,))
     with pytest.raises(SystemExit) as wrapped_error:
         qa_checks.run_qa_checks()
     assert wrapped_error.value.code == 1
@@ -201,7 +201,7 @@ def test_check_missing_migration_guide(mocker, tmp_path, capsys):
     }
     mocker.patch.object(qa_checks.Connector, "metadata", mock_metadata_dict)
 
-    assert qa_checks.check_migration_guide(connector) == False
+    assert qa_checks.check_migration_guide(connector) is False
     stdout, _ = capsys.readouterr()
     assert "Migration guide file is missing for foobar. Please create a foobar-migrations.md file in the docs folder" in stdout
 
@@ -241,6 +241,28 @@ def test_check_invalid_migration_guides(mocker, tmp_path, capsys, test_file, exp
 
     mocker.patch.object(qa_checks.Connector, "metadata", mock_metadata_dict)
 
-    assert qa_checks.check_migration_guide(connector) == False
+    assert qa_checks.check_migration_guide(connector) is False
     stdout, _ = capsys.readouterr()
     assert expected_stdout in stdout
+
+
+def test_get_qa_checks_to_run(mocker):
+    mocker.patch.object(utils.Connector, "has_dockerfile", False)
+    connector = utils.Connector("source-faker")
+
+    assert (
+        qa_checks.get_qa_checks_to_run(connector) == qa_checks.DEFAULT_QA_CHECKS
+    ), "A connector without a Dockerfile should run the default set of QA checks"
+    mocker.patch.object(utils.Connector, "has_dockerfile", True)
+    connector = utils.Connector("source-faker")
+    assert qa_checks.get_qa_checks_to_run(connector) == qa_checks.DEFAULT_QA_CHECKS + (
+        qa_checks.check_metadata_version_matches_dockerfile_label,
+    ), "A connector with a Dockerfile should run the default set of QA checks plus check_metadata_version_matches_dockerfile_label"
+
+
+def test_check_metadata_version_matches_dockerfile_label_without_dockerfile(mocker):
+    mocker.patch.object(utils.Connector, "has_dockerfile", False)
+    connector_without_dockerfile = utils.Connector("source-faker")
+    assert (
+        qa_checks.check_metadata_version_matches_dockerfile_label(connector_without_dockerfile) is False
+    ), "A connector without a Dockerfile should fail check_metadata_version_matches_dockerfile_label"

airbyte-ci/connectors/connector_ops/tests/test_utils.py

@@ -51,8 +51,7 @@ def test_init(self, connector, exists, mocker, tmp_path):
             assert connector.support_level is None
             assert connector.acceptance_test_config is None
             assert connector.icon_path == Path(f"./airbyte-integrations/connectors/{connector.technical_name}/icon.svg")
-            with pytest.raises(FileNotFoundError):
-                connector.version
+            assert connector.version is None
             with pytest.raises(utils.ConnectorVersionNotFound):
                 Path(tmp_path / "Dockerfile").touch()
                 mocker.patch.object(utils.Connector, "code_directory", tmp_path)
@@ -73,6 +72,25 @@ def test_metadata_query_match(self, mocker):
         assert not connector.metadata_query_match("data.ab_internal.ql > 101")
         assert not connector.metadata_query_match("data.ab_internal == whatever")
 
+    @pytest.fixture
+    def connector_without_dockerfile(self, mocker, tmp_path):
+        mocker.patch.object(utils.Connector, "code_directory", tmp_path)
+        connector = utils.Connector("source-faker")
+        return connector
+
+    def test_has_dockerfile_without_dockerfile(self, connector_without_dockerfile):
+        assert not connector_without_dockerfile.has_dockerfile
+
+    @pytest.fixture
+    def connector_with_dockerfile(self, mocker, tmp_path):
+        mocker.patch.object(utils.Connector, "code_directory", tmp_path)
+        connector = utils.Connector("source-faker")
+        tmp_path.joinpath("Dockerfile").touch()
+        return connector
+
+    def test_has_dockerfile_with_dockerfile(self, connector_with_dockerfile):
+        assert connector_with_dockerfile.has_dockerfile
+
 
 @pytest.fixture()
 def gradle_file_with_dependencies(tmpdir) -> tuple[Path, list[Path], list[Path]]:
@@ -158,44 +176,6 @@ def test_parse_dependencies_with_cdk(gradle_file_with_local_cdk_dependencies):
     assert all([test_dependency in expected_test_dependencies for test_dependency in test_dependencies])
 
 
-@pytest.mark.parametrize("with_test_dependencies", [True, False])
-def test_get_all_gradle_dependencies(with_test_dependencies):
-    build_file = Path("airbyte-integrations/connectors/source-postgres-strict-encrypt/build.gradle")
-    if with_test_dependencies:
-        all_dependencies = sorted(utils.get_all_gradle_dependencies(build_file))
-        expected_dependencies = [
-            Path("airbyte-api"),
-            Path("airbyte-cdk/java/airbyte-cdk/core"),
-            Path("airbyte-cdk/java/airbyte-cdk/db-sources"),
-            Path("airbyte-commons"),
-            Path("airbyte-commons-cli"),
-            Path("airbyte-commons-protocol"),
-            Path("airbyte-config-oss/config-models-oss"),
-            Path("airbyte-config-oss/init-oss"),
-            Path("airbyte-connector-test-harnesses/acceptance-test-harness"),
-            Path("airbyte-integrations/bases/base-typing-deduping"),
-            Path("airbyte-integrations/connectors/source-postgres"),
-            Path("airbyte-json-validation"),
-        ]
-        assert set(all_dependencies) == set(expected_dependencies)
-    else:
-        all_dependencies = sorted(utils.get_all_gradle_dependencies(build_file, with_test_dependencies=False))
-        expected_dependencies = [
-            Path("airbyte-api"),
-            Path("airbyte-cdk/java/airbyte-cdk/core"),
-            Path("airbyte-commons"),
-            Path("airbyte-commons-cli"),
-            Path("airbyte-commons-protocol"),
-            Path("airbyte-config-oss/config-models-oss"),
-            Path("airbyte-config-oss/init-oss"),
-            Path("airbyte-connector-test-harnesses/acceptance-test-harness"),
-            Path("airbyte-integrations/bases/base-typing-deduping"),
-            Path("airbyte-integrations/connectors/source-postgres"),
-            Path("airbyte-json-validation"),
-        ]
-        assert set(all_dependencies) == set(expected_dependencies)
-
-
 def test_get_all_connectors_in_repo():
     all_connectors = utils.get_all_connectors_in_repo()
     assert len(all_connectors) > 0