Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

airbyte-ci: build manifest only with local CDK when flag is passed #48818

Merged
merged 5 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions airbyte-ci/connectors/pipelines/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -850,6 +850,7 @@ airbyte-ci connectors --language=low-code migrate-to-manifest-only

| Version | PR | Description |
| ------- | ---------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- |
| 4.44.0 | [#48818](https://github.com/airbytehq/airbyte/pull/48818) | Use local CDK or CDK ref for manifest only connector build. |
| 4.43.1 | [#48824](https://github.com/airbytehq/airbyte/pull/48824) | Allow uploading CI reports to GCS with fewer permissions set. |
| 4.43.0 | [#36545](https://github.com/airbytehq/airbyte/pull/36545) | Switch to `airbyte` user when available in Python base image. |
| 4.42.2 | [#48404](https://github.com/airbytehq/airbyte/pull/48404) | Include `advanced_auth` in spec migration for manifest-only pipeline |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from typing import TYPE_CHECKING

import docker # type: ignore
from click import UsageError
from connector_ops.utils import Connector # type: ignore
from dagger import Container, ExecError, Platform, QueryError
from pipelines.airbyte_ci.connectors.context import ConnectorContext
Expand Down Expand Up @@ -55,7 +56,7 @@ async def _run(self, *args: Any) -> StepResult:
exc_info=e,
)
build_results_per_platform[platform] = connector_container
except QueryError as e:
except (QueryError, UsageError) as e:
return StepResult(
step=self, status=StepStatus.FAILURE, stderr=f"Failed to build connector image for platform {platform}: {e}"
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from pipelines.airbyte_ci.connectors.build_image.steps.common import BuildConnectorImagesBase
from pipelines.airbyte_ci.connectors.context import ConnectorContext
from pipelines.consts import COMPONENTS_FILE_PATH, MANIFEST_FILE_PATH
from pipelines.dagger.actions.python.common import apply_python_development_overrides
from pipelines.models.steps import StepResult
from pydash.objects import get # type: ignore

Expand Down Expand Up @@ -56,8 +57,8 @@ async def _build_from_base_image(self, platform: Platform) -> Container:
f"source_declarative_manifest/{COMPONENTS_FILE_PATH}",
(await self.context.get_connector_dir(include=[COMPONENTS_FILE_PATH])).file(COMPONENTS_FILE_PATH),
)

connector_container = build_customization.apply_airbyte_entrypoint(base_container, self.context.connector)
connector_container = await apply_python_development_overrides(self.context, connector_container)
return connector_container


Expand Down
2 changes: 2 additions & 0 deletions airbyte-ci/connectors/pipelines/pipelines/consts.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@

PUBLISH_UPDATES_SLACK_CHANNEL = "#connector-publish-updates"
PUBLISH_FAILURE_SLACK_CHANNEL = "#connector-publish-failures"
# TODO this should be passed via an env var or a CLI input
PATH_TO_LOCAL_CDK = "../airbyte-python-cdk"


class CIContext(str, Enum):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
from pathlib import Path
from typing import List, Optional, Sequence

from click import UsageError
from dagger import Container, Directory
from pipelines import hacks
from pipelines.airbyte_ci.connectors.context import ConnectorContext, PipelineContext
from pipelines.consts import PATH_TO_LOCAL_CDK
from pipelines.dagger.containers.python import with_pip_cache, with_poetry_cache, with_python_base, with_testing_dependencies
from pipelines.helpers.utils import check_path_in_workdir, get_file_contents

Expand Down Expand Up @@ -238,15 +240,16 @@ def with_python_connector_source(context: ConnectorContext) -> Container:
return with_python_package(context, testing_environment, connector_source_path)


async def apply_python_development_overrides(context: ConnectorContext, connector_container: Container) -> Container:
def apply_python_development_overrides(context: ConnectorContext, connector_container: Container) -> Container:
Copy link
Collaborator

@aaronsteers aaronsteers Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed - this is no longer a coroutine but the return value (Container) is still awaitable. 👍

# Run the connector using the local cdk if flag is set
if context.use_local_cdk:
# Assume CDK is cloned in a sibling dir to `airbyte`:
path_to_cdk = str(Path("../airbyte-python-cdk").resolve())
path_to_cdk = str(Path(PATH_TO_LOCAL_CDK).resolve())
if not Path(path_to_cdk).exists():
raise FileExistsError(f"Local CDK not found at '{path_to_cdk}'")
raise UsageError(
f"Local CDK not found at '{path_to_cdk}'. Please clone the CDK repository in a sibling directory to the airbyte repository. Or use --use-cdk-ref to specify a CDK ref."
)
Comment on lines +249 to +251
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

context.logger.info(f"Using local CDK found at: '{path_to_cdk}'")

directory_to_mount = context.dagger_client.host().directory(path_to_cdk)
cdk_mount_dir = "/airbyte-cdk/python"

Expand Down
2 changes: 1 addition & 1 deletion airbyte-ci/connectors/pipelines/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ build-backend = "poetry.core.masonry.api"

[tool.poetry]
name = "pipelines"
version = "4.43.1"
version = "4.44.0"
description = "Packaged maintained by the connector operations team to perform CI for connectors' pipelines"
authors = ["Airbyte <[email protected]>"]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#

import pytest
from click import UsageError
from pipelines.airbyte_ci.connectors.context import ConnectorContext
from pipelines.dagger.actions.python import common
from pipelines.helpers.connectors.modifed import ConnectorWithModifiedFiles
Expand All @@ -28,24 +29,32 @@ def connector_context(dagger_client):
return context


@pytest.mark.skip(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reason=(
"This is broken since CDK has moved to a separate package. "
"Dagger appears to not have access to the sibling directory. "
+ "See https://github.com/airbytehq/airbyte-internal-issues/issues/10779"
)
)
@pytest.mark.parametrize("use_local_cdk", [True, False])
async def test_apply_python_development_overrides(connector_context, use_local_cdk):
@pytest.mark.parametrize("use_local_cdk, local_cdk_is_available", [(True, True), (True, False), (False, None)])
async def test_apply_python_development_overrides(
dagger_client, mocker, tmp_path, connector_context, use_local_cdk, local_cdk_is_available
):
local_cdk_path = tmp_path / "airbyte-python-cdk"
mocker.patch.object(common, "PATH_TO_LOCAL_CDK", local_cdk_path)
if local_cdk_is_available:
local_cdk_path.mkdir()
await dagger_client.git("https://github.com/airbytehq/airbyte-python-cdk", keep_git_dir=False).branch("main").tree().export(
str(local_cdk_path)
)
connector_context.use_local_cdk = use_local_cdk
fake_connector_container = connector_context.dagger_client.container().from_("airbyte/python-connector-base:2.0.0")
before_override_pip_freeze = await fake_connector_container.with_exec(["pip", "freeze"], use_entrypoint=True).stdout()

assert "airbyte-cdk" not in before_override_pip_freeze.splitlines(), "The base image should not have the airbyte-cdk installed."
connector_with_overrides = await common.apply_python_development_overrides(connector_context, fake_connector_container)

after_override_pip_freeze = await connector_with_overrides.with_exec(["pip", "freeze"], use_entrypoint=True).stdout()
if use_local_cdk:
assert "airbyte-cdk" not in after_override_pip_freeze.splitlines(), "The override should not install the airbyte-cdk package."
if use_local_cdk and not local_cdk_is_available:
# We assume the local cdk is not available so a UsageError should be raised.
with pytest.raises(UsageError):
await common.apply_python_development_overrides(connector_context, fake_connector_container)
else:
assert "airbyte-cdk" not in after_override_pip_freeze.splitlines(), "The override should install the airbyte-cdk package."
overriden_container = await common.apply_python_development_overrides(connector_context, fake_connector_container)
after_override_pip_freeze = await overriden_container.with_exec(["pip", "freeze"], use_entrypoint=True).stdout()

if use_local_cdk and local_cdk_is_available:
assert (
"airbyte-cdk @ file:///airbyte-cdk/python" in after_override_pip_freeze.splitlines()
), "The override should install the airbyte-cdk package."
else:
assert after_override_pip_freeze == before_override_pip_freeze, "The override should not change the pip freeze output."
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
from pathlib import Path

import pytest
from dagger import Container
from pipelines.airbyte_ci.connectors.build_image.steps import build_customization, common, manifest_only_connectors
from pipelines.airbyte_ci.connectors.context import ConnectorContext
from pipelines.airbyte_ci.connectors.build_image.steps import build_customization, manifest_only_connectors
from pipelines.consts import BUILD_PLATFORMS
from pipelines.models.steps import StepStatus
from tests.utils import mock_container
Expand Down Expand Up @@ -99,6 +97,12 @@ async def test__run_using_base_image_with_components_file(
return_value=container_built_from_base,
)

mocker.patch.object(
manifest_only_connectors,
"apply_python_development_overrides",
side_effect=mocker.AsyncMock(return_value=container_built_from_base),
)

step = manifest_only_connectors.BuildConnectorImages(test_context_with_connector_with_base_image)

await step._build_connector(all_platforms[0], container_built_from_base)
Expand Down
6 changes: 6 additions & 0 deletions airbyte-ci/connectors/pipelines/tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ async def with_exec(self, *args, **kwargs):
def with_file(self, *args, **kwargs):
return self

def with_directory(self, *args, **kwargs):
return self

def with_env_variable(self, *args, **kwargs):
return self


def pick_a_random_connector(
language: ConnectorLanguage = None,
Expand Down
Loading