Copilot commented on code in PR #16249:
URL: https://github.com/apache/grails-core/pull/16249#discussion_r3882685981
##########
.github/workflows/forge-deploy-aws.yml:
##########
@@ -13,12 +13,11 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
-# GitHub registers workflow_dispatch workflows from the default branch. Before
-# first deployment, add this workflow at the same path on the default branch,
-# then dispatch it with 7.0.x selected as the run ref.
+# Dispatch from the maintenance branch that should be built (Use workflow
from).
+# Choose the slot. Region, stack name, and JDK come from that branch.
Review Comment:
The comment says region and stack name come from the branch, but the
workflow hard-codes `us-east-1` and `grails-forge-shared` later. Update the
comment to reflect that only the JDK is derived from the selected branch, while
region/stack are fixed.
##########
.github/workflows/forge-deploy-aws.yml:
##########
@@ -60,24 +45,35 @@ jobs:
deploy:
name: "Deploy ${{ inputs.slot }} to AWS Elastic Beanstalk"
runs-on: ubuntu-24.04
- timeout-minutes: 75
+ timeout-minutes: 40
env:
- AWS_REGION: ${{ inputs.aws_region }}
- AWS_DEFAULT_REGION: ${{ inputs.aws_region }}
+ AWS_REGION: us-east-1
+ AWS_DEFAULT_REGION: us-east-1
AWS_PAGER: ""
DEPLOY_ROLE_ARN: ${{ vars.AWS_FORGE_DEPLOY_ROLE_ARN }}
- RELEASE: ${{ inputs.release }}
- SHARED_STACK: ${{ inputs.shared_stack }}
+ SHARED_STACK: grails-forge-shared
SLOT: ${{ inputs.slot }}
steps:
- - name: "Checkout repository"
+ - name: "Checkout"
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd #
v6.0.2
- - name: "Setup Corretto 17"
+ - name: "Read Java version"
+ id: jdk
+ shell: bash
+ run: |
+ set -euo pipefail
+ java_version="$(sed -n 's/^javaVersion=//p' gradle.properties | tr
-d '\r' | awk 'NR==1')"
+ if [[ -z "${java_version}" ]]; then
+ echo "gradle.properties must define javaVersion." >&2
+ exit 1
+ fi
+ printf 'version=%s\n' "${java_version}" >> "${GITHUB_OUTPUT}"
Review Comment:
This step reads `gradle.properties` from the repo root, but later steps run
Gradle with `working-directory: grails-forge`. If `gradle.properties` lives
under `grails-forge/`, this will fail the deploy. Fix by setting
`working-directory: grails-forge` for this step or reading
`grails-forge/gradle.properties` explicitly.
##########
grails-forge/grails-forge-web-netty/build.gradle:
##########
@@ -73,20 +73,23 @@ tasks.named('dockerBuild') {
images = [findProperty('dockerImageName') ?: 'grailsforge']
}
-TaskProvider<ShadowJar> shadowJarTask = tasks.named('shadowJar', ShadowJar)
-shadowJarTask.configure {
- reproducibleFileOrder = true
- preserveFileTimestamps = false
+['com.gradleup.shadow', 'com.github.johnrengelman.shadow'].each { pluginId ->
+ pluginManager.withPlugin(pluginId) {
+ tasks.named('shadowJar', ShadowJar).configure {
+ reproducibleFileOrder = true
+ preserveFileTimestamps = false
+ }
+ }
}
TaskProvider<Zip> awsElasticBeanstalk = tasks.register('awsElasticBeanstalk',
Zip) {
- dependsOn(shadowJarTask)
+ dependsOn('shadowJar')
Review Comment:
Using `tasks.named('shadowJar').get().archiveFile.get()` forces
task/provider realization and loses Gradle’s lazy configuration benefits.
Prefer wiring via providers (e.g., use the
`TaskProvider<ShadowJar>`/`Provider<RegularFile>` form) so the archive file is
resolved lazily and avoids early task realization.
##########
.github/workflows/forge-deploy-aws.yml:
##########
@@ -242,14 +183,8 @@ jobs:
local exit_code=$?
trap - ERR
if [[ "${update_started}" == true && "${rollback_available}" ==
true ]]; then
- echo "Deployment failed. Rolling back ${ENVIRONMENT_NAME} to its
previous application version."
- if aws elasticbeanstalk update-environment --environment-name
"${ENVIRONMENT_NAME}" --version-label "${previous_version}" >/dev/null &&
wait_for_ready_green 'Rollback' "${previous_version}"; then
- echo "Rollback completed."
- else
- echo "Rollback did not reach Status=Ready and Health=Green.
Manual intervention is required." >&2
- fi
- elif [[ "${update_started}" == true ]]; then
- echo "Deployment failed, but no prior application version is
available for rollback." >&2
+ echo "Deployment failed. Rolling back ${ENVIRONMENT_NAME}."
+ aws elasticbeanstalk update-environment --environment-name
"${ENVIRONMENT_NAME}" --version-label "${previous_version}" >/dev/null || true
Review Comment:
Rollback now ignores `update-environment` failures (`|| true`) and no longer
waits/validates that rollback actually returns the environment to a healthy
state. This can leave the environment in a degraded state without clear signal
in the workflow logs. Consider restoring a readiness/health wait (like the
existing `wait_for_ready_green`) and only suppressing errors if you also log
the failure and the environment’s final status/health/version.
##########
grails-forge/grails-forge-web-netty/build.gradle:
##########
@@ -73,20 +73,23 @@ tasks.named('dockerBuild') {
images = [findProperty('dockerImageName') ?: 'grailsforge']
}
-TaskProvider<ShadowJar> shadowJarTask = tasks.named('shadowJar', ShadowJar)
-shadowJarTask.configure {
- reproducibleFileOrder = true
- preserveFileTimestamps = false
+['com.gradleup.shadow', 'com.github.johnrengelman.shadow'].each { pluginId ->
+ pluginManager.withPlugin(pluginId) {
+ tasks.named('shadowJar', ShadowJar).configure {
+ reproducibleFileOrder = true
+ preserveFileTimestamps = false
+ }
+ }
}
TaskProvider<Zip> awsElasticBeanstalk = tasks.register('awsElasticBeanstalk',
Zip) {
- dependsOn(shadowJarTask)
+ dependsOn('shadowJar')
archiveFileName.set('grails-forge-web-netty-aws.zip')
destinationDirectory.set(layout.buildDirectory.dir('distributions'))
reproducibleFileOrder = true
preserveFileTimestamps = false
- from(shadowJarTask.flatMap { it.archiveFile }) {
+ from({ tasks.named('shadowJar').get().archiveFile.get() }) {
rename { 'app.jar' }
}
Review Comment:
Using `tasks.named('shadowJar').get().archiveFile.get()` forces
task/provider realization and loses Gradle’s lazy configuration benefits.
Prefer wiring via providers (e.g., use the
`TaskProvider<ShadowJar>`/`Provider<RegularFile>` form) so the archive file is
resolved lazily and avoids early task realization.
##########
grails-forge/grails-forge-web-netty/build.gradle:
##########
@@ -73,20 +73,23 @@ tasks.named('dockerBuild') {
images = [findProperty('dockerImageName') ?: 'grailsforge']
}
-TaskProvider<ShadowJar> shadowJarTask = tasks.named('shadowJar', ShadowJar)
-shadowJarTask.configure {
- reproducibleFileOrder = true
- preserveFileTimestamps = false
+['com.gradleup.shadow', 'com.github.johnrengelman.shadow'].each { pluginId ->
+ pluginManager.withPlugin(pluginId) {
+ tasks.named('shadowJar', ShadowJar).configure {
+ reproducibleFileOrder = true
+ preserveFileTimestamps = false
Review Comment:
If both shadow plugin IDs are applied (directly or transitively), this will
configure `shadowJar` twice. Consider guarding so the configuration runs once
(e.g., track a boolean flag), or only target the plugin ID(s) that are actually
expected in this build to avoid duplicate configuration and future confusion.
--
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]
