From 917ddae846ee8c9fb2de01412ec2d6368ae75627 Mon Sep 17 00:00:00 2001 From: Augustin Date: Mon, 28 Oct 2024 12:24:44 +0100 Subject: [PATCH] airbyte-ci: rework VersionIncrementCheck for RC (#47386) --- airbyte-ci/connectors/pipelines/README.md | 1 + .../airbyte_ci/connectors/publish/pipeline.py | 2 +- .../connectors/test/steps/common.py | 56 ++++++++++++--- .../connectors/pipelines/pyproject.toml | 2 +- .../tests/test_steps/test_version_check.py | 72 +++++++++++++++++++ 5 files changed, 120 insertions(+), 13 deletions(-) create mode 100644 airbyte-ci/connectors/pipelines/tests/test_steps/test_version_check.py diff --git a/airbyte-ci/connectors/pipelines/README.md b/airbyte-ci/connectors/pipelines/README.md index da75cb2fd326..71190a5823e8 100644 --- a/airbyte-ci/connectors/pipelines/README.md +++ b/airbyte-ci/connectors/pipelines/README.md @@ -850,6 +850,7 @@ airbyte-ci connectors --language=low-code migrate-to-manifest-only | Version | PR | Description | | ------- | ---------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | +| 4.42.0 | [#47386](https://github.com/airbytehq/airbyte/pull/47386) | Version increment check: make sure consecutive RC remain on the same version. | | 4.41.9 | [#47483](https://github.com/airbytehq/airbyte/pull/47483) | Fix build logic used in `up-to-date` to support any connector language. | | 4.41.8 | [#47447](https://github.com/airbytehq/airbyte/pull/47447) | Use `cache_ttl` for base image registry listing in `up-to-date`. | | 4.41.7 | [#47444](https://github.com/airbytehq/airbyte/pull/47444) | Remove redundant `--ignore-connector` error from up-to-date. `--metadata-query` can be used instead. | diff --git a/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/publish/pipeline.py b/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/publish/pipeline.py index 6c3dbbd888ee..56af45b38413 100644 --- a/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/publish/pipeline.py +++ b/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/publish/pipeline.py @@ -19,7 +19,7 @@ from pipelines.airbyte_ci.connectors.build_image import steps from pipelines.airbyte_ci.connectors.publish.context import PublishConnectorContext, RolloutMode from pipelines.airbyte_ci.connectors.reports import ConnectorReport -from pipelines.airbyte_ci.metadata.pipeline import MetadataRollbackReleaseCandidate, MetadataUpload, MetadataValidation +from pipelines.airbyte_ci.metadata.pipeline import MetadataUpload, MetadataValidation from pipelines.airbyte_ci.steps.bump_version import SetConnectorVersion from pipelines.airbyte_ci.steps.changelog import AddChangelogEntry from pipelines.airbyte_ci.steps.pull_request import CreateOrUpdatePullRequest diff --git a/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/steps/common.py b/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/steps/common.py index 316bf75ccef3..1aa0a4f3fc81 100644 --- a/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/steps/common.py +++ b/airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/steps/common.py @@ -13,7 +13,7 @@ from functools import cached_property from pathlib import Path from textwrap import dedent -from typing import Any, ClassVar, Dict, List, Optional, Set +from typing import Any, Dict, List, Optional, Set import dagger import requests # type: ignore @@ -44,7 +44,6 @@ class VersionCheck(Step, ABC): """A step to validate the connector version was bumped if files were modified""" context: ConnectorContext - failure_message: ClassVar @property def should_run(self) -> bool: @@ -79,9 +78,8 @@ def current_connector_version(self) -> semver.Version: def success_result(self) -> StepResult: return StepResult(step=self, status=StepStatus.SUCCESS) - @property - def failure_result(self) -> StepResult: - return StepResult(step=self, status=StepStatus.FAILURE, stderr=self.failure_message) + def _get_failure_result(self, failure_message: str) -> StepResult: + return StepResult(step=self, status=StepStatus.FAILURE, stderr=failure_message) @abstractmethod def validate(self) -> StepResult: @@ -118,10 +116,6 @@ class VersionIncrementCheck(VersionCheck): "build_customization.py", ] - @property - def failure_message(self) -> str: - return f"The dockerImageTag in {METADATA_FILE_NAME} was not incremented. The files you modified should lead to a version bump. Master version is {self.master_connector_version}, current version is {self.current_connector_version}" - @property def should_run(self) -> bool: for filename in self.context.modified_files: @@ -130,9 +124,49 @@ def should_run(self) -> bool: return True return False + def is_version_not_incremented(self) -> bool: + return self.master_connector_version >= self.current_connector_version + + def get_failure_message_for_no_increment(self) -> str: + return ( + f"The dockerImageTag in {METADATA_FILE_NAME} was not incremented. " + f"Master version is {self.master_connector_version}, current version is {self.current_connector_version}" + ) + + def are_both_versions_release_candidates(self) -> bool: + return bool( + self.master_connector_version.prerelease + and self.current_connector_version.prerelease + and "rc" in self.master_connector_version.prerelease + and "rc" in self.current_connector_version.prerelease + ) + + def have_same_major_minor_patch(self) -> bool: + return ( + self.master_connector_version.major == self.current_connector_version.major + and self.master_connector_version.minor == self.current_connector_version.minor + and self.master_connector_version.patch == self.current_connector_version.patch + ) + def validate(self) -> StepResult: - if not self.current_connector_version > self.master_connector_version: - return self.failure_result + if self.is_version_not_incremented(): + return self._get_failure_result( + ( + f"The dockerImageTag in {METADATA_FILE_NAME} was not incremented. " + f"Master version is {self.master_connector_version}, current version is {self.current_connector_version}" + ) + ) + + if self.are_both_versions_release_candidates(): + if not self.have_same_major_minor_patch(): + return self._get_failure_result( + ( + f"Master and current version are release candidates but they have different major, minor or patch versions. " + f"Release candidates should only differ in the prerelease part. Master version is {self.master_connector_version}, " + f"current version is {self.current_connector_version}" + ) + ) + return self.success_result diff --git a/airbyte-ci/connectors/pipelines/pyproject.toml b/airbyte-ci/connectors/pipelines/pyproject.toml index b5044371ce8c..7832ed1a1223 100644 --- a/airbyte-ci/connectors/pipelines/pyproject.toml +++ b/airbyte-ci/connectors/pipelines/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "poetry.core.masonry.api" [tool.poetry] name = "pipelines" -version = "4.41.9" +version = "4.42.0" description = "Packaged maintained by the connector operations team to perform CI for connectors' pipelines" authors = ["Airbyte "] diff --git a/airbyte-ci/connectors/pipelines/tests/test_steps/test_version_check.py b/airbyte-ci/connectors/pipelines/tests/test_steps/test_version_check.py new file mode 100644 index 000000000000..3475fc1d1d9c --- /dev/null +++ b/airbyte-ci/connectors/pipelines/tests/test_steps/test_version_check.py @@ -0,0 +1,72 @@ +# Copyright (c) 2024 Airbyte, Inc., all rights reserved. + +import pytest +from connector_ops.utils import METADATA_FILE_NAME +from pipelines.airbyte_ci.connectors.test.steps.common import VersionIncrementCheck +from semver import VersionInfo + + +class TestVersionIncrementCheck: + @pytest.fixture + def context(self, mocker, tmp_path): + context = mocker.Mock() + context.connector = mocker.Mock(code_directory=str(tmp_path), technical_name="test-connector") + context.modified_files = ["/path/to/connector/src/main.py", "/path/to/connector/README.md"] + context.secrets_to_mask = [] + return context + + def _get_version_increment_check(self, mocker, context, master_version="1.0.0", current_version="1.0.1"): + + mocker.patch( + "pipelines.airbyte_ci.connectors.test.steps.common.VersionIncrementCheck.master_connector_version", + new_callable=mocker.PropertyMock, + return_value=VersionInfo.parse(master_version), + ) + mocker.patch( + "pipelines.airbyte_ci.connectors.test.steps.common.VersionIncrementCheck.current_connector_version", + new_callable=mocker.PropertyMock, + return_value=VersionInfo.parse(current_version), + ) + + return VersionIncrementCheck(context) + + def test_should_run(self, context): + context.modified_files = ["some_file"] + assert VersionIncrementCheck(context).should_run + + def test_should_not_run(self, context): + for bypassed_file in VersionIncrementCheck.BYPASS_CHECK_FOR: + context.modified_files = [bypassed_file] + assert not VersionIncrementCheck(context).should_run + + def test_validate_success_no_rc_increment(self, mocker, context): + version_increment_check = self._get_version_increment_check(mocker, context, master_version="1.0.0", current_version="1.0.1") + result = version_increment_check.validate() + assert result.success + + def test_validate_failure_no_increment(self, context, mocker): + version_increment_check = self._get_version_increment_check(mocker, context, master_version="1.0.0", current_version="1.0.0") + result = version_increment_check.validate() + assert not result.success + assert ( + result.stderr + == f"The dockerImageTag in {METADATA_FILE_NAME} was not incremented. Master version is {version_increment_check.master_connector_version}, current version is {version_increment_check.current_connector_version}" + ) + + def test_validate_failure_rc_with_different_versions(self, context, mocker): + version_increment_check = self._get_version_increment_check( + mocker, context, master_version="1.0.0-rc.1", current_version="1.0.1-rc.1" + ) + result = version_increment_check.validate() + assert not result.success + assert ( + result.stderr + == f"Master and current version are release candidates but they have different major, minor or patch versions. Release candidates should only differ in the prerelease part. Master version is {version_increment_check.master_connector_version}, current version is {version_increment_check.current_connector_version}" + ) + + def test_validate_success_rc_increment(self, context, mocker): + version_increment_check = self._get_version_increment_check( + mocker, context, master_version="1.0.1-rc.1", current_version="1.0.1-rc.2" + ) + result = version_increment_check.validate() + assert result.success