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

Jump to bottom

gradle: remove airbyteDocker.outputs dependencies #30314

Merged
merged 8 commits into from
Sep 12, 2023
Merged

gradle: remove airbyteDocker.outputs dependencies #30314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ffeae03
gradle: improve airbyte-docker performance
postamar Sep 11, 2023
54ebe61
gradle: don't blanket-create 'generate'
postamar Sep 11, 2023
43bf3ac
gradle: airbyte-docker infers dependencies
postamar Sep 11, 2023
bf2c399
gradle: remove all airbyteDocker.outputs dependencies
postamar Sep 11, 2023
01e1f2b
airbyte-ci: remove gradle file patching hacks
postamar Sep 11, 2023
6842402
airbyte-ci: bump version and update changelog
postamar Sep 11, 2023
e0b04ce
gradle: airbyte-docker-ize base-normalization
postamar Sep 11, 2023
dc659e0
airbyte-ci: use integrationTestJava gradle task
postamar Sep 11, 2023
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
16 changes: 15 additions & 1 deletion airbyte-api/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ plugins {

def specFile = "$projectDir/src/main/openapi/config.yaml"

def generate = tasks.register('generate')

// Deprecated -- can be removed once airbyte-server is converted to use the per-domain endpoints generated by generateApiServer
def generateApiServerLegacy = tasks.register('generateApiServerLegacy', GenerateTask) {
def serverOutputDir = "$buildDir/generated/api/server"
Expand Down Expand Up @@ -49,6 +51,9 @@ def generateApiServerLegacy = tasks.register('generateApiServerLegacy', Generate
additionalModelTypeAnnotations: "\n@com.fasterxml.jackson.annotation.JsonInclude(com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL)",
])
}
generate.configure {
dependsOn generateApiServerLegacy
}

def generateApiServer = tasks.register('generateApiServer', GenerateTask) {
def serverOutputDir = "$buildDir/generated/api/server"
Expand Down Expand Up @@ -94,6 +99,9 @@ def generateApiServer = tasks.register('generateApiServer', GenerateTask) {
useTags: "true"
])
}
generate.configure {
dependsOn generateApiServer
}

def generateApiClient = tasks.register('generateApiClient', GenerateTask) {
def clientOutputDir = "$buildDir/generated/api/client"
Expand Down Expand Up @@ -130,6 +138,9 @@ def generateApiClient = tasks.register('generateApiClient', GenerateTask) {
interfaceOnly: "true"
])
}
generate.configure {
dependsOn generateApiClient
}

def generateApiDocs = tasks.register('generateApiDocs', GenerateTask) {
def docsOutputDir = "$buildDir/generated/api/docs"
Expand Down Expand Up @@ -173,6 +184,9 @@ def generateApiDocs = tasks.register('generateApiDocs', GenerateTask) {
}
}
}
generate.configure {
dependsOn generateApiDocs
}

dependencies {
implementation group: 'com.fasterxml.jackson.datatype', name: 'jackson-datatype-jsr310'
Expand All @@ -198,5 +212,5 @@ sourceSets {
}

tasks.named('compileJava').configure {
dependsOn tasks.named('generate')
dependsOn generate
}
8 changes: 6 additions & 2 deletions airbyte-cdk/python/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,16 @@ airbytePython {
moduleDirectory 'airbyte_cdk'
}

tasks.register('generateComponentManifestClassFiles', Exec) {
def generateComponentManifestClassFiles = tasks.register('generateComponentManifestClassFiles', Exec) {
environment 'ROOT_DIR', rootDir.absolutePath
commandLine 'bin/generate-component-manifest-files.sh'
}.configure {
}
generateComponentManifestClassFiles.configure {
dependsOn project(':tools:code-generator').tasks.named('assemble')
}
tasks.register('generate').configure {
dependsOn generateComponentManifestClassFiles
}

tasks.register('validateSourceYamlManifest', Exec) {
environment 'ROOT_DIR', rootDir.absolutePath
Expand Down
1 change: 1 addition & 0 deletions airbyte-ci/connectors/pipelines/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ This command runs the Python tests for a airbyte-ci poetry package.
## Changelog
| Version | PR | Description |
|---------| --------------------------------------------------------- |-----------------------------------------------------------------------------------------------------------|
| 1.1.3 | [#30314](https://github.com/airbytehq/airbyte/pull/30314) | Stop patching gradle files to make them work with airbyte-ci. |
| 1.1.2 | [#30279](https://github.com/airbytehq/airbyte/pull/30279) | Fix correctness issues in layer caching by making atomic execution groupings |
| 1.1.1 | [#30252](https://github.com/airbytehq/airbyte/pull/30252) | Fix redundancies and broken logic in GradleTask, to speed up the CI runs. |
| 1.1.0 | [#29509](https://github.com/airbytehq/airbyte/pull/29509) | Refactor the airbyte-ci test command to run tests on any poetry package. |
Expand Down
2 changes: 1 addition & 1 deletion airbyte-ci/connectors/pipelines/pipelines/contexts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ async def get_connector_dir(self, exclude=None, include=None) -> Directory:
Directory: The connector under test source code directory.
"""
vanilla_connector_dir = self.get_repo_dir(str(self.connector.code_directory), exclude=exclude, include=include)
return await hacks.patch_connector_dir(self, vanilla_connector_dir)
return await vanilla_connector_dir.with_timestamps(1)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alafanechere do we still want this .with_timestamps(1)?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think so: its a trick to avoid cache bursting when a file is rewritten without modification of its content.


async def __aexit__(
self, exception_type: Optional[type[BaseException]], exception_value: Optional[BaseException], traceback: Optional[TracebackType]
Expand Down
59 changes: 1 addition & 58 deletions airbyte-ci/connectors/pipelines/pipelines/hacks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,66 +9,9 @@
from typing import TYPE_CHECKING, Callable, List

import requests
from connector_ops.utils import ConnectorLanguage
from dagger import DaggerError

if TYPE_CHECKING:
from dagger import Client, Container, Directory
from pipelines.contexts import ConnectorContext


LINES_TO_REMOVE_FROM_GRADLE_FILE = [
# Do not build normalization with Gradle - we build normalization with Dagger in the BuildOrPullNormalization step.
"project(':airbyte-integrations:bases:base-normalization').airbyteDocker.output",
]


async def _patch_gradle_file(context: ConnectorContext, connector_dir: Directory) -> Directory:
"""Patch the build.gradle file of the connector under test by removing the lines declared in LINES_TO_REMOVE_FROM_GRADLE_FILE.

Underlying issue:
Java connectors build.gradle declare a dependency to the normalization module.
It means every time we test a java connector the normalization is built.
This is time consuming and not required as normalization is now baked in containers.
Normalization is going away soon so hopefully this hack will be removed soon.

Args:
context (ConnectorContext): The initialized connector context.
connector_dir (Directory): The directory containing the build.gradle file to patch.
Returns:
Directory: The directory containing the patched gradle file.
"""
if context.connector.language is not ConnectorLanguage.JAVA:
context.logger.info(f"Connector language {context.connector.language} does not require a patched build.gradle file.")
return connector_dir

try:
gradle_file_content = await connector_dir.file("build.gradle").contents()
except DaggerError:
context.logger.info("Could not find build.gradle file in the connector directory. Skipping patching.")
return connector_dir

context.logger.warn("Patching build.gradle file to remove normalization build.")

patched_gradle_file = []

for line in gradle_file_content.splitlines():
if not any(line_to_remove in line for line_to_remove in LINES_TO_REMOVE_FROM_GRADLE_FILE):
patched_gradle_file.append(line)
return connector_dir.with_new_file("build.gradle", contents="\n".join(patched_gradle_file))


async def patch_connector_dir(context: ConnectorContext, connector_dir: Directory) -> Directory:
"""Patch a connector directory: patch cat config, gradle file and dockerfile.

Args:
context (ConnectorContext): The initialized connector context.
connector_dir (Directory): The directory containing the connector to patch.
Returns:
Directory: The directory containing the patched connector.
"""
patched_connector_dir = await _patch_gradle_file(context, connector_dir)
return patched_connector_dir.with_timestamps(1)
Comment on lines -16 to -71
Copy link
Contributor

Choose a reason for hiding this comment

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

I love seeing hacks being removed! In order to validate that it's not breaking anything I would love to see a test bhon the following "stable" java connectors:

  • source-postgres, because it's a working source connector
  • destination-bigquery because its a not using normalization
  • destination-snowflake

@edgao @evantahler do you have a destination connector still using normalization which is passing test? I can't find any.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! All green, modulo one small regression fix.

from dagger import Client, Container


async def cache_latest_cdk(dagger_client: Client, pip_cache_volume_name: str = "pip_cache") -> None:
Expand Down
6 changes: 1 addition & 5 deletions airbyte-ci/connectors/pipelines/pipelines/tests/java_connectors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,9 @@
class IntegrationTests(GradleTask):
"""A step to run integrations tests for Java connectors using the integrationTestJava Gradle task."""

gradle_task_name = "integrationTest"
gradle_task_name = "integrationTestJava"
title = "Java Connector Integration Tests"

# skip the connector acceptance tests run by gradle
# as we run them in the AcceptanceTests step
gradle_task_options = ("-x", "connectorAcceptanceTest")

async def _load_normalization_image(self, normalization_tar_file: File):
normalization_image_tag = f"{self.context.connector.normalization_repository}:dev"
self.context.logger.info("Load the normalization image to the docker host.")
Expand Down
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 = "1.1.2"
version = "1.1.3"
description = "Packaged maintained by the connector operations team to perform CI for connectors' pipelines"
authors = ["Airbyte <contact@airbyte.io>"]

Expand Down
3 changes: 3 additions & 0 deletions airbyte-config-oss/config-models-oss/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ jsonSchema2Pojo {
includeSetters = true
serializable = true
}
tasks.register('generate').configure {
dependsOn tasks.named('generateJsonSchema2Pojo')
}

test {
useJUnitPlatform {
Expand Down
1 change: 0 additions & 1 deletion airbyte-integrations/bases/base-java-s3/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ dependencies {
implementation project(':airbyte-config-oss:config-models-oss')
implementation libs.airbyte.protocol
implementation project(':airbyte-integrations:bases:base-java')
implementation files(project(':airbyte-integrations:bases:base-java').airbyteDocker.outputs)
Comment on lines 8 to -9
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice to see these being dropped. 👍


implementation 'org.apache.commons:commons-csv:1.4'
implementation 'com.github.alexmojaki:s3-stream-upload:2.2.2'
Expand Down
6 changes: 0 additions & 6 deletions airbyte-integrations/bases/base-java/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,6 @@ dependencies {
implementation libs.connectors.testcontainers.jdbc
implementation libs.bundles.datadog

implementation files(project(':airbyte-integrations:bases:base').airbyteDocker.outputs)

testImplementation 'commons-lang:commons-lang:2.6'
implementation group: 'org.apache.logging.log4j', name: 'log4j-layout-template-json', version: '2.17.2'
}

tasks.named('airbyteDocker').configure {
dependsOn project(':airbyte-integrations:bases:base').tasks.named('assemble')
}
44 changes: 4 additions & 40 deletions airbyte-integrations/bases/base-normalization/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ checkSshScriptCopy.configure {
dependsOn copySshScript
}

tasks.named('airbyteDocker').configure {
def generate = tasks.register('generate')
generate.configure {
dependsOn checkSshScriptCopy
}

tasks.named('check').configure {
dependsOn checkSshScriptCopy
dependsOn generate
}

def customIntegrationTestPython = tasks.register('customIntegrationTestPython', PythonTask) {
Expand All @@ -62,44 +64,6 @@ static def getImageNameWithTag(String customConnector) {
return "${getDockerImageName(customConnector)}:dev"
}

def buildAirbyteDocker(String customConnector) {
// def baseCommand = ['docker', 'build', '.', '-f', getDockerfile(customConnector), '-t', getImageNameWithTag(customConnector)]
// As the base dbt image (https://hub.docker.com/r/fishtownanalytics/dbt/tags) we are using is only build for amd64, we need to use buildkit to force builds for your local environment
// We are lucky that all the python code dbt uses is mutli-arch compatible

def arch = 'linux/amd64'
if (Os.isArch("aarch_64") || Os.isArch("aarch64")) {
arch = 'linux/arm64'
}

def cmdArray = ['docker', 'buildx', 'build', '--load', '--platform', arch, '-f', getDockerfile(customConnector), '-t', getImageNameWithTag(customConnector), '.']
// println("Building normalization container: " + cmdArray.join(" "))

return {
commandLine cmdArray
}
}

[ 'airbyteDockerMSSql': 'mssql',
'airbyteDockerMySql': 'mysql',
'airbyteDockerOracle': 'oracle',
'airbyteDockerClickhouse': 'clickhouse',
'airbyteDockerSnowflake': 'snowflake',
'airbyteDockerRedshift': 'redshift',
'airbyteDockerTiDB': 'tidb',
'airbyteDockerDuckDB': 'duckdb'
].forEach {taskName, customConnector ->
def task = tasks.register(taskName, Exec) {
configure buildAirbyteDocker(customConnector)
}
task.configure {
dependsOn checkSshScriptCopy
}
tasks.named('assemble').configure {
dependsOn task
}
}

def customIntegrationTestsCoverage = tasks.named('_customIntegrationTestsCoverage')
customIntegrationTestsCoverage.configure {
dependsOn tasks.named('assemble')
Expand Down
3 changes: 0 additions & 3 deletions airbyte-integrations/bases/bases-destination-jdbc/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,4 @@ dependencies {

integrationTestJavaImplementation project(':airbyte-integrations:bases:standard-destination-test')
integrationTestJavaImplementation libs.connectors.testcontainers.postgresql

implementation files(project(':airbyte-integrations:bases:base-java').airbyteDocker.outputs)
integrationTestJavaImplementation files(project(':airbyte-integrations:bases:base-normalization').airbyteDocker.outputs)
Comment on lines -32 to -34
Copy link
Collaborator

Choose a reason for hiding this comment

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

I mentioned this in our 1:1, but I’m excited to see these drop because they complicated the Java CDK build process. Happy to see them retiring.

}
3 changes: 3 additions & 0 deletions airbyte-integrations/bases/standard-source-test/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ def generateSourceTestDocs = tasks.register('generateSourceTestDocs', Javadoc) {

outputs.upToDateWhen { false }
}
tasks.register('generate').configure {
dependsOn generateSourceTestDocs
}

application {
mainClass = 'io.airbyte.integrations.standardtest.source.PythonSourceAcceptanceTest'
Expand Down
3 changes: 0 additions & 3 deletions airbyte-integrations/connector-templates/source-java-jdbc/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,4 @@ dependencies {

integrationTestJavaImplementation project(':airbyte-integrations:connectors:source-{{dashCase name}}')
integrationTestJavaImplementation project(':airbyte-integrations:bases:standard-source-test')

implementation files(project(':airbyte-integrations:bases:base-java').airbyteDocker.outputs)
integrationTestJavaImplementation files(project(':airbyte-integrations:bases:base-java').airbyteDocker.outputs)
}
1 change: 0 additions & 1 deletion airbyte-integrations/connectors-performance/destination-harness/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ dependencies {
implementation libs.airbyte.protocol
implementation 'io.fabric8:kubernetes-client:5.12.2'
implementation 'org.apache.commons:commons-lang3:3.11'
implementation files(project(':airbyte-integrations:bases:base-java').airbyteDocker.outputs)
implementation 'io.airbyte:airbyte-commons-worker:0.42.0'
implementation 'io.airbyte.airbyte-config:config-models:0.42.0'
implementation 'junit:junit:4.13.1'
Expand Down
1 change: 0 additions & 1 deletion airbyte-integrations/connectors-performance/source-harness/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ dependencies {
implementation libs.airbyte.protocol
implementation 'io.fabric8:kubernetes-client:5.12.2'
implementation 'org.apache.commons:commons-lang3:3.11'
implementation files(project(':airbyte-integrations:bases:base-java').airbyteDocker.outputs)
implementation 'io.airbyte:airbyte-commons-worker:0.42.0'
implementation 'io.airbyte.airbyte-config:config-models:0.42.0'
}
1 change: 0 additions & 1 deletion airbyte-integrations/connectors/destination-azure-blob-storage/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ dependencies {
implementation libs.airbyte.protocol
implementation project(':airbyte-integrations:bases:base-java')
implementation project(':airbyte-integrations:bases:bases-destination-jdbc')
implementation files(project(':airbyte-integrations:bases:base-java').airbyteDocker.outputs)

implementation 'com.azure:azure-storage-blob:12.20.2'
implementation 'org.apache.commons:commons-csv:1.4'
Expand Down
1 change: 0 additions & 1 deletion airbyte-integrations/connectors/destination-bigquery-denormalized/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ dependencies {
integrationTestJavaImplementation project(':airbyte-integrations:connectors:destination-bigquery-denormalized')
integrationTestJavaImplementation project(':airbyte-db:db-lib')

implementation files(project(':airbyte-integrations:bases:base-java').airbyteDocker.outputs)
}

configurations.all {
Expand Down
1 change: 0 additions & 1 deletion airbyte-integrations/connectors/destination-bigquery/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ dependencies {
integrationTestJavaImplementation project(':airbyte-integrations:connectors:destination-bigquery')
integrationTestJavaImplementation project(':airbyte-db:db-lib')

implementation files(project(':airbyte-integrations:bases:base-java').airbyteDocker.outputs)
}

configurations.all {
Expand Down
1 change: 0 additions & 1 deletion airbyte-integrations/connectors/destination-cassandra/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ dependencies {
implementation project(':airbyte-config-oss:config-models-oss')
implementation libs.airbyte.protocol
implementation project(':airbyte-integrations:bases:base-java')
implementation files(project(':airbyte-integrations:bases:base-java').airbyteDocker.outputs)

implementation "com.datastax.oss:java-driver-core:${cassandraDriver}"
implementation "com.datastax.oss:java-driver-query-builder:${cassandraDriver}"
Expand Down
8 changes: 1 addition & 7 deletions airbyte-integrations/connectors/destination-clickhouse-strict-encrypt/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ dependencies {
implementation libs.airbyte.protocol
implementation project(':airbyte-integrations:bases:bases-destination-jdbc')
implementation project(':airbyte-integrations:connectors:destination-clickhouse')
implementation files(project(':airbyte-integrations:bases:base-java').airbyteDocker.outputs)

implementation 'com.clickhouse:clickhouse-jdbc:0.3.2-patch10:all'

Expand All @@ -26,9 +25,4 @@ dependencies {
integrationTestJavaImplementation project(':airbyte-integrations:connectors:destination-clickhouse')
// https://mvnrepository.com/artifact/org.testcontainers/clickhouse
integrationTestJavaImplementation libs.connectors.destination.testcontainers.clickhouse
integrationTestJavaImplementation files(project(':airbyte-integrations:bases:base-normalization').airbyteDocker.outputs)
}

tasks.named('airbyteDocker').configure {
dependsOn project(':airbyte-integrations:bases:base-normalization').tasks.named('airbyteDockerClickhouse')
}
}
2 changes: 0 additions & 2 deletions airbyte-integrations/connectors/destination-clickhouse/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ dependencies {
implementation libs.airbyte.protocol
implementation project(':airbyte-integrations:bases:base-java')
implementation project(':airbyte-integrations:bases:bases-destination-jdbc')
implementation files(project(':airbyte-integrations:bases:base-java').airbyteDocker.outputs)

implementation 'com.clickhouse:clickhouse-jdbc:0.3.2-patch10:all'

Expand All @@ -28,7 +27,6 @@ dependencies {
integrationTestJavaImplementation project(':airbyte-connector-test-harnesses:acceptance-test-harness')
// https://mvnrepository.com/artifact/org.testcontainers/clickhouse
integrationTestJavaImplementation libs.connectors.destination.testcontainers.clickhouse
integrationTestJavaImplementation files(project(':airbyte-integrations:bases:base-normalization').airbyteDocker.outputs)
}

tasks.named("airbyteDocker").configure {
Expand Down
1 change: 0 additions & 1 deletion airbyte-integrations/connectors/destination-csv/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ dependencies {
implementation project(':airbyte-config-oss:config-models-oss')
implementation libs.airbyte.protocol
implementation project(':airbyte-integrations:bases:base-java')
implementation files(project(':airbyte-integrations:bases:base-java').airbyteDocker.outputs)

integrationTestJavaImplementation project(':airbyte-integrations:bases:standard-destination-test')
}
Loading
Loading