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.

Sign up for GitHub

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: refactor GradleTask to run in subprocess #52033

Merged
merged 1 commit into from
Jan 22, 2025
Merged

airbyte-ci: refactor GradleTask to run in subprocess #52033

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 29 additions & 0 deletions .github/actions/install-java-environment/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
name: Install Java Environment
description: "Installs the Java environment"
inputs:
java_version:
description: "Java version"
required: false
default: "21"
type: string
gradle_cache_read_only:
description: "Whether to use a read-only Gradle cache"
required: false
default: false
type: boolean
gradle_cache_write_only:
description: "Whether to use a write-only Gradle cache"
required: false
default: false
type: boolean
runs:
using: "composite"
steps:
- uses: actions/setup-java@v4
with:
distribution: corretto
java-version: ${{ inputs.java_version }}
- uses: gradle/actions/setup-gradle@v3
with:
cache-read-only: ${{ inputs.gradle_cache_read_only }}
cache-write-only: ${{ inputs.gradle_cache_write_only }}
4 changes: 3 additions & 1 deletion .github/actions/run-airbyte-ci/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,9 @@ runs:
ls -la
ls -la airbyte-python-cdk || echo "No airbyte-python-cdk directory"
ls -laL ../airbyte-python-cdk || echo "No airbyte-python-cdk symlink"
- name: Install Java Environment
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For a followup: that would be cool to spare python connector only PR from installing the java env. We could rely on the changes output to check if any java connector was modified

davinchia marked this conversation as resolved.
Show resolved Hide resolved
id: install-java-environment
uses: ./.github/actions/install-java-environment
- name: Docker login
id: docker-login
uses: docker/login-action@v3
Expand Down
3 changes: 2 additions & 1 deletion airbyte-ci/connectors/pipelines/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -854,8 +854,9 @@ airbyte-ci connectors --language=low-code migrate-to-manifest-only

| Version | PR | Description |
| ------- | ---------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- |
| 4.49.0 | [#52033](https://github.com/airbytehq/airbyte/pull/52033) | Run gradle as a subprocess and not via Dagger |
| 4.48.9 | [#51609](https://github.com/airbytehq/airbyte/pull/51609) | Fix ownership of shared cache volume for non root connectors |
| 4.48.8 | [#51582](https://github.com/airbytehq/airbyte/pull/51582) | Fix typo in `migrate-to-inline-schemas` command |
| 4.48.8 | [#51582](https://github.com/airbytehq/airbyte/pull/51582) | Fix typo in `migrate-to-inline-schemas` command |
| 4.48.7 | [#51579](https://github.com/airbytehq/airbyte/pull/51579) | Give back the ownership of /tmp to the original user on finalize build |
| 4.48.6 | [#51577](https://github.com/airbytehq/airbyte/pull/51577) | Run finalize build scripts as root |
| 4.48.5 | [#49827](https://github.com/airbytehq/airbyte/pull/49827) | Bypasses CI checks for promoted release candidate PRs. |
Expand Down
3 changes: 2 additions & 1 deletion ...connectors/pipelines/pipelines/airbyte_ci/connectors/build_image/steps/java_connectors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ async def run_connector_build(context: ConnectorContext) -> StepResult:
build_connector_tar_result = await BuildConnectorDistributionTar(context).run()
if build_connector_tar_result.status is not StepStatus.SUCCESS:
return build_connector_tar_result
dist_dir = await build_connector_tar_result.output.directory(dist_tar_directory_path(context))

dist_dir = await build_connector_tar_result.output.directory("build/distributions")
Copy link
Contributor

@davinchia davinchia Jan 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

for understanding: why are we using "build/distributions" directly? feels unrelated to the other changes, maybe I'm missing something.

Copy link
Contributor Author

@alafanechere alafanechere Jan 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It changes because the original path related to an "absolute path airbyte repo path". the build_connector_tar step now returns a directory which is the connector directory.

return await BuildConnectorImages(context).run(dist_dir)


Expand Down
4 changes: 1 addition & 3 deletions ...yte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/steps/java_connectors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,7 @@ def get_test_steps(context: ConnectorTestContext) -> STEP_TREE:
StepToRun(
id=CONNECTOR_TEST_STEP_ID.BUILD,
step=BuildConnectorImages(context),
args=lambda results: {
"dist_dir": results[CONNECTOR_TEST_STEP_ID.BUILD_TAR].output.directory(dist_tar_directory_path(context))
},
args=lambda results: {"dist_dir": results[CONNECTOR_TEST_STEP_ID.BUILD_TAR].output.directory("build/distributions")},
depends_on=[CONNECTOR_TEST_STEP_ID.BUILD_TAR],
),
],
Expand Down
389 changes: 191 additions & 198 deletions airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/steps/gradle.py
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion airbyte-ci/connectors/pipelines/pyproject.toml
View file
Open in desktop
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.48.9"
version = "4.49.0"
description = "Packaged maintained by the connector operations team to perform CI for connectors' pipelines"
authors = ["Airbyte <contact@airbyte.io>"]

Expand Down
21 changes: 0 additions & 21 deletions airbyte-ci/connectors/pipelines/tests/test_dagger/test_actions/test_python/test_common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,24 +61,3 @@ def context_with_setup(dagger_client, python_connector_with_setup_not_latest_cdk
@pytest.fixture(scope="module")
def python_connector_base_image_address(python_connector_with_setup_not_latest_cdk):
return python_connector_with_setup_not_latest_cdk.metadata["connectorBuildOptions"]["baseImage"]


async def test_with_python_connector_installed_from_setup(context_with_setup, python_connector_base_image_address, latest_cdk_version):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test was failing - removing it as installing from setup.py is now legacy

python_container = context_with_setup.dagger_client.container().from_(python_connector_base_image_address)
user = await BuildConnectorImages.get_image_user(python_container)
container = await common.with_python_connector_installed(
context_with_setup, python_container, str(context_with_setup.connector.code_directory), user
)
# Uninstall and reinstall the latest cdk version
cdk_install_latest_output = (
await container.with_env_variable("CACHEBUSTER", datetime.datetime.now().isoformat())
.with_user("root")
.with_exec(["pip", "uninstall", "-y", f"airbyte-cdk=={latest_cdk_version}"])
.with_exec(["pip", "install", f"airbyte-cdk=={latest_cdk_version}"])
.stdout()
)
# Assert that the latest cdk version is installed from cache
assert f"Using cached airbyte_cdk-{latest_cdk_version}-py3-none-any.whl" in cdk_install_latest_output
# Assert that the connector is installed
pip_freeze_output = await container.with_exec(["pip", "freeze"]).stdout()
assert context_with_setup.connector.technical_name in pip_freeze_output
4 changes: 0 additions & 4 deletions airbyte-ci/connectors/pipelines/tests/test_gradle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ def test_context(self, mocker, dagger_client):
),
)

async def test_build_include(self, test_context):
step = self.DummyStep(test_context)
assert step.build_include

def test_params(self, test_context):
step = self.DummyStep(test_context)
step.extra_params = {"-x": ["dummyTask", "dummyTask2"]}
Expand Down
Loading