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: format improvements #32999

Merged
merged 16 commits into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -30,25 +30,4 @@ async def run_format(
format_commands (List[str]): The list of commands to run to format the repository
"""
format_container = container.with_exec(sh_dash_c(format_commands), skip_entrypoint=True)
return await format_container.directory("/src").export(".")


def mount_repo_for_formatting(
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 was not called anywhere.

dagger_client: dagger.Client,
container: dagger.Container,
include: List[str],
) -> dagger.Container:
"""Mounts the relevant parts of the repository: the code to format and the formatting config
Args:
container: (dagger.Container): The container to mount the repository in
include (List[str]): The list of files to include in the container
"""
container = container.with_mounted_directory(
"/src",
dagger_client.host().directory(
".",
include=include,
),
).with_workdir("/src")

return container
return await format_container.directory("/airbyte/repo").export(".")
postamar marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ async def python(pipeline_context: ClickPipelineContext) -> CommandResult:
dagger_client = await pipeline_context.get_dagger_client(pipeline_name="Check python formatting")
container = format_python_container(dagger_client)
check_commands = [
"poetry install --no-root",
"poetry run isort --settings-file pyproject.toml --check-only .",
"poetry run black --config pyproject.toml --check .",
]
Expand All @@ -77,10 +76,10 @@ async def python(pipeline_context: ClickPipelineContext) -> CommandResult:
@check.command(cls=DaggerPipelineCommand)
@pass_pipeline_context
async def java(pipeline_context: ClickPipelineContext) -> CommandResult:
"""Format java, groovy, and sql code via spotless."""
"""Format java code via spotless in maven."""
postamar marked this conversation as resolved.
Show resolved Hide resolved
postamar marked this conversation as resolved.
Show resolved Hide resolved
dagger_client = await pipeline_context.get_dagger_client(pipeline_name="Check java formatting")
container = format_java_container(dagger_client)
check_commands = ["./gradlew spotlessCheck --scan"]
check_commands = ["mvn -f spotless-maven-pom.xml spotless:check"]
return await get_check_command_result(check.commands["java"], check_commands, container)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@

import dagger
from pipelines.airbyte_ci.format.consts import DEFAULT_FORMAT_IGNORE_LIST
from pipelines.consts import AMAZONCORRETTO_IMAGE, GO_IMAGE, NODE_IMAGE, PYTHON_3_10_IMAGE
from pipelines.consts import GO_IMAGE, MAVEN_IMAGE, NODE_IMAGE, PYTHON_3_10_IMAGE
from pipelines.helpers.utils import sh_dash_c


def build_container(
dagger_client: dagger.Client,
base_image: str,
include: List[str],
warmup_include: Optional[List[str]] = None,
postamar marked this conversation as resolved.
Show resolved Hide resolved
install_commands: Optional[List[str]] = None,
env_vars: Optional[Dict[str, Any]] = {},
) -> dagger.Container:
Expand All @@ -22,6 +23,7 @@ def build_container(
ctx (ClickPipelineContext): The context of the pipeline
base_image (str): The base image to use for the container
include (List[str]): The list of files to include in the container
warmup_include (Optional[List[str]]): The list of files to include in the container before installing dependencies
install_commands (Optional[List[str]]): The list of commands to run to install dependencies for the formatter
env_vars (Optional[Dict[str, Any]]): The list of environment variables to set on the container
Returns:
Expand All @@ -34,6 +36,22 @@ def build_container(
for key, value in env_vars.items():
container = container.with_env_variable(key, value)

# Set the working directory to the code to format
container = container.with_workdir("/src")
postamar marked this conversation as resolved.
Show resolved Hide resolved

# Mount a subset of the relevant parts of the repository, if requested.
# These should only be files which do not change very often.
# These can then be referenced by the install_commands.
postamar marked this conversation as resolved.
Show resolved Hide resolved
if warmup_include:
container = container.with_mounted_directory(
postamar marked this conversation as resolved.
Show resolved Hide resolved
"/src",
dagger_client.host().directory(
".",
include=warmup_include,
exclude=DEFAULT_FORMAT_IGNORE_LIST,
),
)

# Install any dependencies of the formatter
if install_commands:
container = container.with_exec(sh_dash_c(install_commands), skip_entrypoint=True)
Expand All @@ -44,39 +62,36 @@ def build_container(
"/src",
dagger_client.host().directory(
".",
postamar marked this conversation as resolved.
Show resolved Hide resolved
include=include,
include=include + (warmup_include if warmup_include else []),
exclude=DEFAULT_FORMAT_IGNORE_LIST,
),
)

# Set the working directory to the code to format
container = container.with_workdir("/src")

return container


def format_java_container(dagger_client: dagger.Client) -> dagger.Container:
"""Format java, groovy, and sql code via spotless."""
"""Format java code via spotless in maven."""
return build_container(
dagger_client,
base_image=AMAZONCORRETTO_IMAGE,
include=[
"**/*.java",
"**/*.gradle",
"gradlew",
"gradlew.bat",
"gradle",
"**/deps.toml",
"**/gradle.properties",
"**/version.properties",
base_image=MAVEN_IMAGE,
warmup_include=[
"spotless-maven-pom.xml",
"tools/gradle/codestyle/java-google-style.xml",
],
install_commands=[
"yum update -y",
"yum install -y findutils", # gradle requires xargs, which is shipped in findutils.
"yum clean all",
# Run maven before mounting the sources to download all its dependencies.
# Dagger will cache the resulting layer to minimize the downloads.
# The go-offline goal purportedly downloads all dependencies.
# This isn't quite the case, we still need to add spotless goals.
"mvn -f spotless-maven-pom.xml"
" org.apache.maven.plugins:maven-dependency-plugin:3.6.1:go-offline"
" spotless:apply"
" spotless:check",
],
include=[
"**/*.java",
],
env_vars={"RUN_IN_AIRBYTE_CI": "1"},
postamar marked this conversation as resolved.
Show resolved Hide resolved
)


Expand All @@ -101,11 +116,16 @@ def format_license_container(dagger_client: dagger.Client, license_file: str) ->

def format_python_container(dagger_client: dagger.Client) -> dagger.Container:
"""Format python code via black and isort."""

return build_container(
dagger_client,
base_image=PYTHON_3_10_IMAGE,
env_vars={"PIPX_BIN_DIR": "/usr/local/bin"},
include=["**/*.py", "pyproject.toml", "poetry.lock"],
install_commands=["pip install pipx", "pipx ensurepath", "pipx install poetry"],
warmup_include=["pyproject.toml", "poetry.lock"],
install_commands=[
"pip install pipx",
"pipx ensurepath",
"pipx install poetry",
"poetry install --no-root",
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 should speed up the python formatting by caching the poetry install which depends on a poetry.lock file which changes very little over time.

],
include=["**/*.py"],
)
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ async def all_fix(ctx: click.Context):
@pass_pipeline_context
@click_ignore_unused_kwargs
async def java(ctx: ClickPipelineContext) -> CommandResult:
"""Format java, groovy, and sql code via spotless."""
"""Format java code via spotless in maven."""
dagger_client = await ctx.get_dagger_client(pipeline_name="Format java")
container = format_java_container(dagger_client)
format_commands = ["./gradlew spotlessApply --scan"]
format_commands = ["mvn -f spotless-maven-pom.xml spotless:apply"]
erohmensing marked this conversation as resolved.
Show resolved Hide resolved
return await get_format_command_result(fix.commands["java"], container, format_commands)


Expand Down Expand Up @@ -127,7 +127,6 @@ async def python(ctx: ClickPipelineContext) -> CommandResult:
dagger_client = await ctx.get_dagger_client(pipeline_name="Format python")
container = format_python_container(dagger_client)
format_commands = [
"poetry install --no-root",
"poetry run isort --settings-file pyproject.toml .",
"poetry run black --config pyproject.toml .",
]
Expand Down
1 change: 1 addition & 0 deletions airbyte-ci/connectors/pipelines/pipelines/consts.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
NODE_IMAGE = "node:18.18.0-slim"
GO_IMAGE = "golang:1.17"
PYTHON_3_10_IMAGE = "python:3.10.13-slim"
MAVEN_IMAGE = "maven:3.9.5-amazoncorretto-17-al2023"
DOCKER_VERSION = "24.0.2"
DOCKER_DIND_IMAGE = f"docker:{DOCKER_VERSION}-dind"
DOCKER_CLI_IMAGE = f"docker:{DOCKER_VERSION}-cli"
Expand Down
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ allprojects {
def javaTarget = createFormatTarget('**/*.java')
if (!javaTarget.isEmpty()) {
java {
// See also spotless-maven-pom.xml which is used by airbyte-ci, its config should be in sync with this one.
postamar marked this conversation as resolved.
Show resolved Hide resolved
target javaTarget
importOrder()
eclipse('4.21').configFile(rootProject.file('tools/gradle/codestyle/java-google-style.xml'))
Expand Down
42 changes: 42 additions & 0 deletions spotless-maven-pom.xml
postamar marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?xml version="1.0" encoding="UTF-8"?>

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<groupId>io.airbyte</groupId>
<artifactId>airbyte-spotless-format-dummy-maven-project</artifactId>
<version>1.0-SNAPSHOT</version>
<name>airbyte-spotless-format-dummy-maven-project</name>

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<java.version>17</java.version>
</properties>

<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>com.diffplug.spotless</groupId>
<artifactId>spotless-maven-plugin</artifactId>
<version>2.41.0</version>
<configuration>
<java>
<includes>
<include>**/*.java</include>
</includes>
<importOrder />
<eclipse>
<version>4.21</version>
<file>tools/gradle/codestyle/java-google-style.xml</file>
</eclipse>
<removeUnusedImports />
<trimTrailingWhitespace />
</java>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>
</project>
postamar marked this conversation as resolved.
Show resolved Hide resolved
Loading