diff --git a/airbyte-ci/connectors/connector_ops/connector_ops/qa_checks.py b/airbyte-ci/connectors/connector_ops/connector_ops/qa_checks.py index 462a05c1c4d0..03ac01c3a4c6 100644 --- a/airbyte-ci/connectors/connector_ops/connector_ops/qa_checks.py +++ b/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(): diff --git a/airbyte-ci/connectors/connector_ops/connector_ops/utils.py b/airbyte-ci/connectors/connector_ops/connector_ops/utils.py index 32bf07a2dd1e..0578e7631bf9 100644 --- a/airbyte-ci/connectors/connector_ops/connector_ops/utils.py +++ b/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: diff --git a/airbyte-ci/connectors/connector_ops/pyproject.toml b/airbyte-ci/connectors/connector_ops/pyproject.toml index 53104d9cdd52..8ccedea1c4d8 100644 --- a/airbyte-ci/connectors/connector_ops/pyproject.toml +++ b/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 "] diff --git a/airbyte-ci/connectors/connector_ops/tests/test_qa_checks.py b/airbyte-ci/connectors/connector_ops/tests/test_qa_checks.py index a7860b65505a..2c647ad9258b 100644 --- a/airbyte-ci/connectors/connector_ops/tests/test_qa_checks.py +++ b/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" diff --git a/airbyte-ci/connectors/connector_ops/tests/test_utils.py b/airbyte-ci/connectors/connector_ops/tests/test_utils.py index b4f6ca7746cb..1f8d3f931cad 100644 --- a/airbyte-ci/connectors/connector_ops/tests/test_utils.py +++ b/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])