make QA checks tolerate connector without dockerfile

Author: alafanechere

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
 from pydash.objects import get
@@ -238,7 +238,7 @@ def check_metadata_version_matches_dockerfile_label(connector: Connector) -> boo
     return connector.version_in_dockerfile_label == 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.
@@ -250,8 +250,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:
@@ -285,7 +290,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

@@ -236,6 +236,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
@@ -253,22 +257,20 @@ def language(self) -> ConnectorLanguage:
             return ConnectorLanguage.LOW_CODE
         if Path(self.code_directory / "setup.py").is_file():
             return ConnectorLanguage.PYTHON
-        try:
-            with open(self.code_directory / "Dockerfile") as dockerfile:
-                if "FROM airbyte/integration-base-java" in dockerfile.read():
-                    return ConnectorLanguage.JAVA
-        except FileNotFoundError:
-            pass
+        if Path(self.code_directory / "build.gradle").is_file():
+            return ConnectorLanguage.JAVA
         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) -> str:
+    def version_in_dockerfile_label(self) -> Optional[str]:
+        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:

airbyte-ci/connectors/connector_ops/pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "poetry.core.masonry.api"
 
 [tool.poetry]
 name = "connector_ops"
-version = "0.2.3"
+version = "0.2.4"
 description = "Packaged maintained by the connector operations team to perform CI for connectors"
 authors = ["Airbyte <[email protected]>"]
 

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) -> Path:
@@ -105,49 +123,3 @@ def test_parse_dependencies(gradle_file_with_dependencies):
     assert all([regular_dependency in expected_regular_dependencies for regular_dependency in regular_dependencies])
     assert len(test_dependencies) == len(expected_test_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 = utils.get_all_gradle_dependencies(build_file)
-        expected_dependencies = [
-            Path("airbyte-cdk/java/airbyte-cdk"),
-            Path("airbyte-db/db-lib"),
-            Path("airbyte-json-validation"),
-            Path("airbyte-config-oss/config-models-oss"),
-            Path("airbyte-commons"),
-            Path("airbyte-test-utils"),
-            Path("airbyte-api"),
-            Path("airbyte-connector-test-harnesses/acceptance-test-harness"),
-            Path("airbyte-commons-protocol"),
-            Path("airbyte-integrations/bases/base-java"),
-            Path("airbyte-commons-cli"),
-            Path("airbyte-integrations/bases/base"),
-            Path("airbyte-integrations/connectors/source-postgres"),
-            Path("airbyte-integrations/bases/debezium"),
-            Path("airbyte-integrations/connectors/source-jdbc"),
-            Path("airbyte-integrations/connectors/source-relational-db"),
-            Path("airbyte-integrations/bases/standard-source-test"),
-        ]
-        assert len(all_dependencies) == len(expected_dependencies)
-        assert all([dependency in expected_dependencies for dependency in all_dependencies])
-    else:
-        all_dependencies = utils.get_all_gradle_dependencies(build_file, with_test_dependencies=False)
-        expected_dependencies = [
-            Path("airbyte-cdk/java/airbyte-cdk"),
-            Path("airbyte-db/db-lib"),
-            Path("airbyte-json-validation"),
-            Path("airbyte-config-oss/config-models-oss"),
-            Path("airbyte-commons"),
-            Path("airbyte-integrations/bases/base-java"),
-            Path("airbyte-commons-cli"),
-            Path("airbyte-integrations/bases/base"),
-            Path("airbyte-integrations/connectors/source-postgres"),
-            Path("airbyte-integrations/bases/debezium"),
-            Path("airbyte-integrations/connectors/source-jdbc"),
-            Path("airbyte-integrations/connectors/source-relational-db"),
-        ]
-        assert len(all_dependencies) == len(expected_dependencies)
-        assert all([dependency in expected_dependencies for dependency in all_dependencies])