jason810496 commented on code in PR #68380: URL: https://github.com/apache/airflow/pull/68380#discussion_r3400625772
########## java-sdk/plugin/src/main/kotlin/org/apache/airflow/sdk/plugin/AirflowSdkPlugin.kt: ########## @@ -0,0 +1,218 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.airflow.sdk.plugin + +import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar +import org.gradle.api.Plugin Review Comment: Needed for the `GradleException` used in the suggestion on `verifyBundleMainClass` below. ```suggestion import org.gradle.api.GradleException import org.gradle.api.Plugin ``` ########## java-sdk/README.md: ########## @@ -125,13 +148,14 @@ To test the full publish flow without touching ASF infrastructure, override the repository URL to a local directory ```bash -./gradlew :sdk:publish -PmavenUrl=file:///tmp/local-maven-repo -PskipSigning -ls /tmp/local-maven-repo/org/apache/airflow/airflow-sdk/ +./gradlew publish -PmavenUrl=file:///tmp/local-maven-repo -PskipSigning=true +ls /tmp/local-maven-repo/org/apache/airflow/ +# This should contain the same components in ~/.m2 as inspected in the previous step, Review Comment: ```suggestion # This should contain the same components in ~/.m2 as inspected in the previous step. ``` ########## java-sdk/plugin/src/main/kotlin/org/apache/airflow/sdk/plugin/AirflowSdkPlugin.kt: ########## @@ -0,0 +1,218 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.airflow.sdk.plugin + +import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar +import org.gradle.api.Plugin +import org.gradle.api.Project +import org.gradle.api.plugins.JavaPluginExtension +import org.gradle.api.provider.Property +import org.gradle.api.tasks.Copy +import org.gradle.api.tasks.Input +import org.gradle.api.tasks.bundling.Jar +import java.lang.reflect.Modifier +import java.net.URLClassLoader +import java.util.jar.JarFile +import kotlin.jvm.java + +/** + * DSL extension registered by [AirflowSdkPlugin] as the `airflowBundle` block. + * + * ```kotlin + * airflowBundle { + * mainClass = "com.example.ExampleBundleBuilder" + * // fatJar = false // opt out of shadow JAR creation + * } + * ``` + */ +abstract class AirflowBundleExtension { + /** Fully qualified name of the entry-point class. + * + * This is written to the JAR manifest under the standard `Main-Class` key. The + * coordinator uses the value to execute the bundle JAR correctly. + */ + @get:Input + abstract val mainClass: Property<String> + + /** + * Whether to build a fat JAR using the Shadow plugin (default: `true`). + * + * When `true`, the Shadow plugin is applied automatically and + * `Airflow-Supervisor-Schema-Version` is injected into the shadow JAR manifest. + * When `false`, no fat JAR is produced; only `Main-Class` is set on the + * regular `jar` task. + */ + @get:Input + abstract val fatJar: Property<Boolean> +} + +/** + * Gradle plugin for building Apache Airflow Java SDK bundles. + * + * Minimal usage: + * + * ```kotlin + * plugins { + * id("org.apache.airflow.sdk") version <version> + * } + * + * airflowBundle { + * mainClass = "com.example.ExampleBundleBuilder" + * } + * ``` + * + * See [AirflowBundleExtension] to understand how to configure the plugin with the + * `airflowBundle` block. + * + * The plugin automatically sets the `Main-Class` metadata, and provides a new + * task `bundle` to create a Dag bundle in one command. This builds deploy-ready + * artifacts to `build/bundle/` that can be copied directly into an Airflow Java + * coordinator's `jars_root`. + * + * By default, plugin `com.github.johnrengelman.shadow` is applied automatically + * to enable fat JAR build. In this mode, one single JAR with user code and all + * dependencies is produced by the `bundle` task. An additional metadata entry + * `Airflow-Supervisor-Schema-Version` is also added to the JAR for Airflow to + * identify which version of the Supervisor Schema it should use to communicate + * with the built JAR. + * + * If `fatJar` is explicitly set to `false`, the `bundle` task builds a bare JAR + * containing only the Dag bundle, and collect all dependency JARs into the target + * directory instead. In this mode, `Airflow-Supervisor-Schema-Version` lives in + * the `airflow-sdk` JAR instead. The bundle JAR still contains `Main-Class`. Review Comment: Does it mean that we need to update the `JavaCoordinator` to respect the thin jar deployment approach? IIUC, the current `JavaCoordinator` discovery logic only handle the fatJar. ########## java-sdk/plugin/src/main/kotlin/org/apache/airflow/sdk/plugin/AirflowSdkPlugin.kt: ########## @@ -0,0 +1,218 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.airflow.sdk.plugin + +import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar +import org.gradle.api.Plugin +import org.gradle.api.Project +import org.gradle.api.plugins.JavaPluginExtension +import org.gradle.api.provider.Property +import org.gradle.api.tasks.Copy +import org.gradle.api.tasks.Input +import org.gradle.api.tasks.bundling.Jar +import java.lang.reflect.Modifier +import java.net.URLClassLoader +import java.util.jar.JarFile +import kotlin.jvm.java + +/** + * DSL extension registered by [AirflowSdkPlugin] as the `airflowBundle` block. + * + * ```kotlin + * airflowBundle { + * mainClass = "com.example.ExampleBundleBuilder" + * // fatJar = false // opt out of shadow JAR creation + * } + * ``` + */ +abstract class AirflowBundleExtension { + /** Fully qualified name of the entry-point class. + * + * This is written to the JAR manifest under the standard `Main-Class` key. The + * coordinator uses the value to execute the bundle JAR correctly. + */ + @get:Input + abstract val mainClass: Property<String> + + /** + * Whether to build a fat JAR using the Shadow plugin (default: `true`). + * + * When `true`, the Shadow plugin is applied automatically and + * `Airflow-Supervisor-Schema-Version` is injected into the shadow JAR manifest. + * When `false`, no fat JAR is produced; only `Main-Class` is set on the + * regular `jar` task. + */ + @get:Input + abstract val fatJar: Property<Boolean> +} + +/** + * Gradle plugin for building Apache Airflow Java SDK bundles. + * + * Minimal usage: + * + * ```kotlin + * plugins { + * id("org.apache.airflow.sdk") version <version> + * } + * + * airflowBundle { + * mainClass = "com.example.ExampleBundleBuilder" + * } + * ``` + * + * See [AirflowBundleExtension] to understand how to configure the plugin with the + * `airflowBundle` block. + * + * The plugin automatically sets the `Main-Class` metadata, and provides a new + * task `bundle` to create a Dag bundle in one command. This builds deploy-ready + * artifacts to `build/bundle/` that can be copied directly into an Airflow Java + * coordinator's `jars_root`. + * + * By default, plugin `com.github.johnrengelman.shadow` is applied automatically + * to enable fat JAR build. In this mode, one single JAR with user code and all + * dependencies is produced by the `bundle` task. An additional metadata entry + * `Airflow-Supervisor-Schema-Version` is also added to the JAR for Airflow to + * identify which version of the Supervisor Schema it should use to communicate + * with the built JAR. + * + * If `fatJar` is explicitly set to `false`, the `bundle` task builds a bare JAR + * containing only the Dag bundle, and collect all dependency JARs into the target + * directory instead. In this mode, `Airflow-Supervisor-Schema-Version` lives in + * the `airflow-sdk` JAR instead. The bundle JAR still contains `Main-Class`. + * + */ +class AirflowSdkPlugin : Plugin<Project> { + override fun apply(project: Project) { + project.plugins.apply("java") + + val ext = project.extensions.create("airflowBundle", AirflowBundleExtension::class.java) + ext.fatJar.convention(true) + + project.afterEvaluate { + project.tasks.withType(Jar::class.java).configureEach { task -> + task.doFirst { + task.manifest.attributes(mapOf("Main-Class" to ext.mainClass.get())) + } + } + + val classFiles = + project.objects.fileCollection().from( + project.extensions + .getByType(JavaPluginExtension::class.java) + .sourceSets + .getByName("main") + .output + .classesDirs, + project.configurations.getByName("runtimeClasspath"), + ) + val className = ext.mainClass.get() + + val verifyTask = + project.tasks.register("verifyBundleMainClass") { task -> + task.group = "verification" + task.description = + "Verifies that mainClass exists in the compiled output and declares a public static main method." + task.dependsOn(project.tasks.named("classes")) + task.inputs.files(classFiles).withPropertyName("classFiles") + task.inputs.property("mainClass", className) + task.doLast { + val urls = classFiles.map { it.toURI().toURL() }.toTypedArray() Review Comment: **More context by Claude:** Second half of the lazy `mainClass` change: drop the eager `get()` and fail only here, with a message that names the DSL block to add. The `orElse("")` on the input property matters because Gradle fingerprints task inputs *before* execution — an absent provider would reproduce the same opaque "Cannot query the value" error before `doLast` can raise the clear one. `bundle` depends on this task in both the fat- and thin-JAR branches, so a bundle can never be produced without a valid `Main-Class` regardless of task ordering. ```suggestion val verifyTask = project.tasks.register("verifyBundleMainClass") { task -> task.group = "verification" task.description = "Verifies that mainClass exists in the compiled output and declares a public static main method." task.dependsOn(project.tasks.named("classes")) task.inputs.files(classFiles).withPropertyName("classFiles") // orElse keeps input fingerprinting from failing on the absent // provider before doLast can raise the actionable error below. task.inputs.property("mainClass", mainClass.orElse("")) task.doLast { val className = mainClass.orNull ?: throw GradleException( "airflowBundle.mainClass must be set to build an Airflow bundle:\n" + "airflowBundle {\n" + " mainClass = \"com.example.Main\"\n" + "}", ) val urls = classFiles.map { it.toURI().toURL() }.toTypedArray() ``` ########## java-sdk/plugin/src/main/kotlin/org/apache/airflow/sdk/plugin/AirflowSdkPlugin.kt: ########## @@ -0,0 +1,218 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.airflow.sdk.plugin + +import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar +import org.gradle.api.Plugin +import org.gradle.api.Project +import org.gradle.api.plugins.JavaPluginExtension +import org.gradle.api.provider.Property +import org.gradle.api.tasks.Copy +import org.gradle.api.tasks.Input +import org.gradle.api.tasks.bundling.Jar +import java.lang.reflect.Modifier +import java.net.URLClassLoader +import java.util.jar.JarFile +import kotlin.jvm.java + +/** + * DSL extension registered by [AirflowSdkPlugin] as the `airflowBundle` block. + * + * ```kotlin + * airflowBundle { + * mainClass = "com.example.ExampleBundleBuilder" + * // fatJar = false // opt out of shadow JAR creation + * } + * ``` + */ +abstract class AirflowBundleExtension { + /** Fully qualified name of the entry-point class. + * + * This is written to the JAR manifest under the standard `Main-Class` key. The + * coordinator uses the value to execute the bundle JAR correctly. + */ + @get:Input + abstract val mainClass: Property<String> + + /** + * Whether to build a fat JAR using the Shadow plugin (default: `true`). + * + * When `true`, the Shadow plugin is applied automatically and + * `Airflow-Supervisor-Schema-Version` is injected into the shadow JAR manifest. + * When `false`, no fat JAR is produced; only `Main-Class` is set on the + * regular `jar` task. + */ + @get:Input + abstract val fatJar: Property<Boolean> +} + +/** + * Gradle plugin for building Apache Airflow Java SDK bundles. + * + * Minimal usage: + * + * ```kotlin + * plugins { + * id("org.apache.airflow.sdk") version <version> + * } + * + * airflowBundle { + * mainClass = "com.example.ExampleBundleBuilder" + * } + * ``` + * + * See [AirflowBundleExtension] to understand how to configure the plugin with the + * `airflowBundle` block. + * + * The plugin automatically sets the `Main-Class` metadata, and provides a new + * task `bundle` to create a Dag bundle in one command. This builds deploy-ready + * artifacts to `build/bundle/` that can be copied directly into an Airflow Java + * coordinator's `jars_root`. + * + * By default, plugin `com.github.johnrengelman.shadow` is applied automatically + * to enable fat JAR build. In this mode, one single JAR with user code and all + * dependencies is produced by the `bundle` task. An additional metadata entry + * `Airflow-Supervisor-Schema-Version` is also added to the JAR for Airflow to + * identify which version of the Supervisor Schema it should use to communicate + * with the built JAR. + * + * If `fatJar` is explicitly set to `false`, the `bundle` task builds a bare JAR + * containing only the Dag bundle, and collect all dependency JARs into the target + * directory instead. In this mode, `Airflow-Supervisor-Schema-Version` lives in + * the `airflow-sdk` JAR instead. The bundle JAR still contains `Main-Class`. + * + */ +class AirflowSdkPlugin : Plugin<Project> { + override fun apply(project: Project) { + project.plugins.apply("java") + + val ext = project.extensions.create("airflowBundle", AirflowBundleExtension::class.java) + ext.fatJar.convention(true) + + project.afterEvaluate { + project.tasks.withType(Jar::class.java).configureEach { task -> + task.doFirst { + task.manifest.attributes(mapOf("Main-Class" to ext.mainClass.get())) + } + } Review Comment: This was caught by Claude and I verified with the following steps: ``` # on this feature branch cd java-sdk ./gradlew publishToMavenLocal -PskipSigning=true # should work fine with mainClass set cd example ../gradlew help # comment out the mainClass line in java-sdk/example/build.gradle: airflowBundle { // mainClass = "org.apache.airflow.example.ExampleBundleBuilder" } # Re-run the same unrelated task: ../gradlew help ``` Then I got the following error after comment out the `mainClass` in `java-sdk/example/build.gradle`: ``` Calculating task graph as configuration cache cannot be reused because file 'build.gradle' has changed. [Incubating] Problems report is available at: file:///Users/jason/airflow-workspace/java-sdk/example/build/reports/problems/problems-report.html FAILURE: Build failed with an exception. * What went wrong: A problem occurred configuring root project 'airflow-java-sdk-example'. > Cannot query the value of extension 'airflowBundle' property 'mainClass' because it has no value available. * Try: > Run with --stacktrace option to get the stack trace. > Run with --info or --debug option to get more log output. > Run with --scan to get full insights. > Get more help at https://help.gradle.org. Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0. You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins. For more on this, please refer to https://docs.gradle.org/8.14.4/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation. BUILD FAILED in 363ms Configuration cache entry stored. ``` --- **More context by Claude:** Applying the plugin without setting `mainClass` currently breaks *every* task. Even with`./gradlew help`, `test`, IDE sync at configuration time, because `afterEvaluate` calls `ext.mainClass.get()` eagerly (here in the `Jar` hook, and at `val className` below). Suggest wiring it lazily: capture the `Property` and skip the manifest attribute when unset, so only `verifyBundleMainClass`/`bundle` fail (with an actionable message — see the companion suggestion below). Capturing the `Property` rather than `ext` also keeps the task-action lambdas configuration-cache serializable. One behavior note: with this change a plain `jar` invocation silently produces a JAR without `Main-Class`. That seems right — `bundle` is the deploy artifact and remains guarded by `verifyBundleMainClass` — but flagging in case you'd rather warn there. ```suggestion ext.fatJar.convention(true) val mainClass = ext.mainClass project.afterEvaluate { project.tasks.withType(Jar::class.java).configureEach { task -> task.doFirst { // Only the bundle flow needs Main-Class; when it is unset, // verifyBundleMainClass fails with an actionable message instead. mainClass.orNull?.let { main -> task.manifest.attributes(mapOf("Main-Class" to main)) } } } ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
