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

Ludo java21 #9882

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

Ludo java21 #9882

wants to merge 19 commits into from

Conversation

ludoch
Copy link
Contributor

@ludoch ludoch commented Dec 16, 2024

Description

Move all GAE java8 runtime (which is now EOL) to the latest GAE java21 runtime, using the compatible EE8 setting.
This way, the java source code should not change (i.e same servlets code), and only the xml config files, like appengine-web.xml are changed.
Manually tested with local devappserver as well as deployed to my cloud project.

Cannot submit without doing the Google public pages changes seen in pending cl/691482072 internally.
I am coordinating with the Google Cloud doc folks.

Checklist

  • I have followed Sample Format Guide
  • pom.xml parent set to latest shared-configuration
  • Appropriate changes to README are included in PR
  • These samples need a new API enabled in testing projects to pass (let us know which ones)
  • These samples need a new/updated env vars in testing projects set to pass (let us know which ones)
  • Tests pass: mvn clean verify required
  • Lint passes: mvn -P lint checkstyle:check required
  • Static Analysis: mvn -P lint clean compile pmd:cpd-check spotbugs:check advisory only
  • This sample adds a new sample directory, and I updated the CODEOWNERS file with the codeowners for this sample
  • This sample adds a new Product API, and I updated the Blunderbuss issue/PR auto-assigner with the codeowners for this sample
  • Please merge this PR for me once it is approved

@product-auto-label product-auto-label bot added samples Issues that are directly related to samples. api: bigtable Issues related to the Bigtable API. labels Dec 16, 2024
Copy link
Contributor

Gemini encountered an error creating the summary. You can try again by commenting @code-review-assist summarize.

@ludoch
Copy link
Contributor Author

ludoch commented Dec 17, 2024

@code-review-assist summarize

Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

Hi @ludoch, Gemini is currently reviewing this pull request and will post a full review in a few minutes. In the meantime, here's a quick summary for you and other reviewers:

This pull request migrates all Java 8 App Engine samples to the latest Java 21 runtime. The goal is to update the samples to a supported runtime, while minimizing changes to the core servlet code. The primary changes involve modifying the appengine-web.xml files in each sample to specify the Java 21 runtime and using the compatible EE8 setting. Additionally, several README.md files have been updated to reflect the Java 21 runtime. The pom.xml files in several samples have also been updated to reflect the new artifact IDs.

Specifically, the following files were modified:

  • appengine-java21/README.md
  • appengine-java21/analytics/README.md, appengine-java21/analytics/pom.xml, appengine-java21/analytics/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/appidentity/README.md, appengine-java21/appidentity/pom.xml, appengine-java21/appidentity/src/main/java/com/example/appengine/appidentity/IdentityServlet.java, appengine-java21/appidentity/src/main/java/com/example/appengine/appidentity/SignForAppServlet.java, appengine-java21/appidentity/src/main/java/com/example/appengine/appidentity/UrlShortener.java, appengine-java21/appidentity/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/bigquery/README.md, appengine-java21/bigquery/pom.xml, appengine-java21/bigquery/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/bigtable/README.md, appengine-java21/bigtable/build.gradle, appengine-java21/bigtable/pom.xml, appengine-java21/bigtable/settings.gradle, appengine-java21/bigtable/src/main/webapp/WEB-INF/appengine-web.xml, appengine-java21/bigtable/src/main/webapp/WEB-INF/logging.properties, appengine-java21/bigtable/src/main/webapp/bigtable.jsp
  • appengine-java21/cloudsql/README.md
  • appengine-java21/datastore-indexes-exploding/pom.xml, appengine-java21/datastore-indexes-exploding/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/datastore-indexes-perfect/pom.xml, appengine-java21/datastore-indexes-perfect/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/datastore-indexes/pom.xml, appengine-java21/datastore-indexes/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/datastore-schedule-export/cron.yaml, appengine-java21/datastore-schedule-export/src/main/webapp/WEB-INF/appengine-web.xml, appengine-java21/datastore-schedule-export/src/main/webapp/WEB-INF/logging.properties, appengine-java21/datastore-schedule-export/src/main/webapp/index.jsp
  • appengine-java21/datastore/README.md, appengine-java21/datastore/pom.xml, appengine-java21/datastore/src/main/webapp/WEB-INF/appengine-web.xml, appengine-java21/datastore/src/test/java/com/example/appengine/QueriesTest.java
  • appengine-java21/firebase-tictactoe/README.md, appengine-java21/firebase-tictactoe/pom.xml, appengine-java21/firebase-tictactoe/src/main/webapp/WEB-INF/appengine-web.xml, appengine-java21/firebase-tictactoe/src/main/webapp/WEB-INF/logging.properties
  • appengine-java21/gaeinfo/README.md, appengine-java21/gaeinfo/pom.xml, appengine-java21/gaeinfo/src/main/webapp/WEB-INF/appengine-web.xml, appengine-java21/gaeinfo/src/main/webapp/WEB-INF/logging.properties
  • appengine-java21/guestbook-cloud-datastore/README.md, appengine-java21/guestbook-cloud-datastore/pom.xml, appengine-java21/guestbook-cloud-datastore/src/main/webapp/WEB-INF/appengine-web.xml, appengine-java21/guestbook-cloud-datastore/src/main/webapp/WEB-INF/index.yaml, appengine-java21/guestbook-cloud-datastore/src/main/webapp/WEB-INF/logging.properties
  • appengine-java21/iap/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/images/README.md, appengine-java21/images/pom.xml, appengine-java21/images/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/mail/pom.xml, appengine-java21/mail/src/main/webapp/WEB-INF/appengine-web.xml, appengine-java21/mail/src/main/webapp/WEB-INF/logging.properties
  • appengine-java21/mailgun/README.md, appengine-java21/mailgun/pom.xml, appengine-java21/mailgun/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/mailjet/README.md, appengine-java21/mailjet/pom.xml, appengine-java21/mailjet/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/memcache/pom.xml, appengine-java21/memcache/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/metadata/README.md, appengine-java21/metadata/pom.xml, appengine-java21/metadata/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/oauth2/README.md, appengine-java21/oauth2/pom.xml, appengine-java21/oauth2/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/pubsub/README.md, appengine-java21/pubsub/src/main/java/com/example/appengine/pubsub/PubSubAuthenticatedPush.java, appengine-java21/pubsub/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/remote-README.md, appengine-java21/remote-client/pom.xml, appengine-java21/remote-server/pom.xml, appengine-java21/remote-server/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/requests/README.md, appengine-java21/requests/pom.xml, appengine-java21/requests/src/main/java/com/example/appengine/requests/LoggingServlet.java, appengine-java21/requests/src/main/java/com/example/appengine/requests/RequestsServlet.java, appengine-java21/requests/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/search/README.md, appengine-java21/search/pom.xml, appengine-java21/search/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/sendgrid/README.md, appengine-java21/sendgrid/pom.xml, appengine-java21/sendgrid/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/spanner/README.md, appengine-java21/spanner/pom.xml, appengine-java21/spanner/src/main/webapp/WEB-INF/appengine-web.xml
  • appengine-java21/sparkjava-helloworld/README.md, appengine-java21/sparkjava-helloworld/pom.xml, appengine-java21/sparkjava-helloworld/src/main/java/HelloWorld.java, appengine-java21/sparkjava-helloworld/src/main/webapp/WEB-INF/appengine-web.xml, appengine-java21/sparkjava-helloworld/src/main/webapp/WEB-INF/logging.properties
  • appengine-java21/springboot-helloworld/.mvn/wrapper/maven-wrapper.properties, appengine-java21/springboot-helloworld/lombok.config, appengine-java21/springboot-helloworld/mvnw, appengine-java21/springboot-helloworld/mvnw.cmd, appengine-java21/springboot-helloworld/pom.xml, appengine-java21/springboot-helloworld/src/main/java/com/example/appengine/demos/springboot/AppEngineConfig.java, appengine-java21/springboot-helloworld/src/main/java/com/example/appengine/demos/springboot/HelloworldController.java, appengine-java21/springboot-helloworld/src/main/java/com/example/appengine/demos/springboot/ServletInitializer.java, appengine-java21/springboot-helloworld/src/main/java/com/example/appengine/demos/springboot/SpringBootExampleApplication.java, appengine-java21/springboot-helloworld/src/main/java/com/example/appengine/demos/springboot/WebAppConfiguration.java, appengine-java21/springboot-helloworld/src/main/resources/application.yml, appengine-java21/springboot-helloworld/src/main/resources/logging.properties, appengine-java21/springboot-helloworld/src/main/webapp/WEB-INF/appengine-web.xml, appengine-java21/springboot-helloworld/src/main/webapp/WEB-INF/datastore-indexes.xml, appengine-java21/springboot-helloworld/src/main/webapp/WEB-INF/jsp/index.jsp, appengine-java21/springboot-helloworld/src/main/webapp/WEB-INF/web.xml, appengine-java21/springboot-helloworld/src/test/java/com/example/appengine/demos/springboot/SpringBootExampleApplicationTests.java
  • appengine-java21/static-files/pom.xml, appengine-java21/static-files/src/main/webapp/WEB-INF/appengine-web.xml, appengine-java21/static-files/src/main/webapp/index.html
  • appengine-java21/taskqueues-deferred/README.md, appengine-java21/taskqueues-deferred/pom.xml, appengine-java21/taskqueues-deferred/src/main/webapp/WEB-INF/appengine-web.xml, appengine-java21/taskqueues-deferred/src/main/webapp/WEB-INF/logging.properties
  • appengine-java21/taskqueues-pull/README.md, appengine-java21/taskqueues-pull/pom.xml, appengine-java21/taskqueues-pull/src/main/webapp/WEB-INF/appengine-web.xml, appengine-java21/taskqueues-pull/src/main/webapp/WEB-INF/logging.properties
  • appengine-java21/taskqueues-push/README.md, appengine-java21/taskqueues-push/pom.xml, appengine-java21/taskqueues-push/src/main/webapp/WEB-INF/appengine-web.xml

This is a significant update, so thorough testing is crucial. Let me know if you have any questions.

Small changes, big impact,
Java's new version's a pact,
Modernizing code's art.

@ludoch ludoch removed the api: bigtable Issues related to the Bigtable API. label Dec 17, 2024
@ludoch ludoch marked this pull request as ready for review December 17, 2024 23:01
@ludoch ludoch requested review from yoshi-approver and a team as code owners December 17, 2024 23:01
@ludoch
Copy link
Contributor Author

ludoch commented Dec 17, 2024

We need to wait that cl/691482072 is also done, and will need to coordinate the 2 changes at the same time.

@ludoch
Copy link
Contributor Author

ludoch commented Dec 17, 2024

Also, I cannot see any more information about the check "Inclusive Language Completed in 21s — Check failed due to error"
Where is the real output?

Copy link

snippet-bot bot commented Dec 17, 2024

Here is the summary of possible violations 😱

There are 145 possible violations for not having product prefix.
There are 19 possible violations for removing region tag in use.
There is a format violation for a region tag.

The end of the violation section. All the stuff below is FYI purposes only.


You are about to delete the following sample browser pages.

You are about to delete the following frozen region tags.

Here is the summary of changes.

You are about to add 247 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@iennae
Copy link
Contributor

iennae commented Dec 17, 2024

looks like we need to fix [ERROR] Failed to execute goal on project gaeinfo-j21: Could not resolve dependencies for project com.example.appengine:gaeinfo-j21:war:1.0-SNAPSHOT: Could not transfer artifact com.squareup.okhttp3:okhttp:jar:4.12.0 from/to central (https://repo.maven.apache.org/maven2): GET request of: com/squareup/okhttp3/okhttp/4.12.0/okhttp-4.12.0.jar from central failed: Connection reset -> [Help 1]

ludoch and others added 2 commits January 9, 2025 05:03
* chore(job): migrate regions by associating them with an official product with a job_ prefix (#9883)

* chore(endpoints): delete region 'swagger' in endpoints/multiple-versions (#9857)

* chore(endpoints): delete region swagger to openapi-v1.yaml

* chore(endpoints): delete region swagger to openapi-v2.yaml

* chore(job): delete sample jobs_java_dependencies_beta (#9810)

* chore(job): delete sample jobs_java_dependencies_beta

* chore(job): delete region_tab 'jobs_java_dependencies_beta' and update 'google-api-services-jobs' version

* feat(compute): add compute disk regional replicated sample (#9697)

* Implemented compute_disk_regional_replicated sample, created test

* Fixed zone

* Fixed test

* Fixed test

* Fixed disk size

* Fixed code as requested in the comment

* feat(compute): add compute disk start/stop replication samples  (#9650)

* Implemented compute_disk_start_replication and compute_disk_stop_replication samples, created tests

* Fixed test

* Deleted not related classes

* Fixed lint issue

* Increased timeout

* Split samples for zonal location

* Fixed code

* Fixed code

* Increased timeout

* Increased timeout

* feat(tpu): add tpu vm create spot sample. (#9610)

* Changed package, added information to CODEOWNERS

* Added information to CODEOWNERS

* Added timeout

* Fixed parameters for test

* Fixed DeleteTpuVm and naming

* Added comment, created Util class

* Fixed naming

* Fixed whitespace

* Split PR into smaller, deleted redundant code

* Implemented tpu_vm_create_spot sample, created test

* changed zone

* Changed zone

* Fixed empty lines and tests, deleted cleanup method

* Changed zone

* Deleted redundant test class

* Increased timeout

* Fixed test

* feat(tpu): add tpu vm create startup script sample. (#9612)

* Changed package, added information to CODEOWNERS

* Added information to CODEOWNERS

* Added timeout

* Fixed parameters for test

* Fixed DeleteTpuVm and naming

* Added comment, created Util class

* Fixed naming

* Fixed whitespace

* Split PR into smaller, deleted redundant code

* Implemented tpu_vm_create_startup_script sample, created test

* Fixed tests and empty lines

* Changed zone

* Deleted redundant test classes

* Increased timeout

* Fixed code

* feat(tpu): add tpu queued resources create/get/delete  samples (#9613)

* Changed package, added information to CODEOWNERS

* Added information to CODEOWNERS

* Added timeout

* Fixed parameters for test

* Fixed DeleteTpuVm and naming

* Added comment, created Util class

* Fixed naming

* Fixed whitespace

* Split PR into smaller, deleted redundant code

* Implemented tpu_queued_resources_create, tpu_queued_resources_get, tpu_queued_resources_delete_force and tpu_queued_resources_delete samples, created tests

* Fixed test

* Fixed tests

* Fixed error massage

* Fixed typo

* Fixed zone

* Fixed test

* Fixed code

* Deleted commented imports

* Fixed code as requested in comments

* feat(tpu): add tpu queued resources create spot (#9615)

Add a code sample for tpu_queued_resources_create_spot

* chore: add translate dev team for translate samples (#9888)

b/385243174

* feat(securitycenter): Add Resource SCC Management API Org ETD Custom Module code samples (Create, Delete, List, Get) (#9743)

* sample codes for event threat detection custom modules

* addressed comments

* addressed comments

* addressed comments

* addressed comments

* fix(compute): fixed compute_reservation_create_shared sample and test to use mocked client (#9840)

* Fixed sample and test to use mocked client

* Fixed code as requested in the comments

* feat(compute): add compute instance create replicated boot disk sample (#9735)

* Implemented compute_instance_create_replicated_boot_disk sample, created test

* Fixed test

* Fixed code as requested in the comments

* Fixed Util class

* Fixed code

* feat(compute): add compute consistency group stop replication (#9694)

* Implemented compute_consistency_group_create and compute_consistency_group_delete samples, created test

* Implemented compute_consistency_group_stop_replication sample

* Implemented compute_consistency_group_stop_replication sample

* Created test and added needed classes for testing

* Fixed test

* Moved clean up methods

* Added clean up methods for reservations

* Fixed clean up method

* Fixed clean up method

* Added timeout

* Reverted not related changes

* Reverted not related changes

* Reverted not related changes

* Reverted not related changes

* Fixed code

* Split samples for zonal location

* Added comments for methods

* Fixed comments

* feat(secretmanager): add optional ttl to create secret sample (#9889)

* feat(secretmanager): add optional ttl to create secret sample

* nit: Update secretmanager/src/main/java/secretmanager/CreateSecret.java

Co-authored-by: code-review-assist[bot] <182814678+code-review-assist[bot]@users.noreply.github.com>

* fix(secretmanager): fix comment indentation to resolve linting issues

---------

Co-authored-by: Jennifer Davis <[email protected]>
Co-authored-by: code-review-assist[bot] <182814678+code-review-assist[bot]@users.noreply.github.com>

* feat(tpu): add tpu  queued resources list sample (#9614)

* Changed package, added information to CODEOWNERS

* Added information to CODEOWNERS

* Added timeout

* Fixed parameters for test

* Fixed DeleteTpuVm and naming

* Added comment, created Util class

* Fixed naming

* Fixed whitespace

* Split PR into smaller, deleted redundant code

* Implemented tpu_queued_resources_create, tpu_queued_resources_get, tpu_queued_resources_delete_force and tpu_queued_resources_delete samples, created tests

* Implemented tpu_queued_resources_list sample, created test

* Fixed test

* Fixed tests, deleted cleanup method

* Fixed test

* Fixed imports

* feat(compute): add compute disk create secondary regional sample (#9641)

* Implemented compute_disk_create_secondary_regional. created test

* Fixed test

* Fixed test

* Fixed test

* Fixed zone

* Fixed naming

* Fixed spaces

* Fixed code

* Fixed indentations

* Fixed variable

* Fixed code

* Added cleanup methods

* Fixed lint issue

* Fixed lint issue

* Fixed test

* Fixed code

* Fixed code

* Fixed code

* Deleted duplicated assertion

* feat(compute): add compute disk create secondary sample. (#9643)

* Implemented compute_disk_create_secondary sample, created test

* Fixed code

* Fixed variable

* Fixed code

* Merged changes from main

* Fixed lint issue

* fix(storage): migrate old region all to storagetransfer_transfer_all step 1 (#9917)

* fix(job): remove old region create_job (#9914)

* feat(compute): attach/ remove snapshot schedule to disk (#9791)

* Implemented compute_snapshot_schedule_attach sample, created test

* Implemented compute_snapshot_schedule_remove sample, created test

* Fixed code

* Fixed code as requested in the comments

* feat(compute): add compute consistency group clone sample (#9885)

* Implemented compute_consistency_group_clone and compute_consistency_group_clone_regional_disk samples, created tests

* Fixed naming

* feat(compute): add compute instance attach regional disk force sample (#9730)

* Implemented compute_instance_attach_regional_disk_force sample, created test

* Added clean up method

* Fixed comments and parameters

* Test order deleted

* Fixed code

* Fixed code

* Fixed code

* Increased timeout

* Increased timeout

* Increased timeout

* Fixed code

* Fixed code

* Fixed code

* Fixed naming

* feat(compute): add compute disk create secondary custom sample (#9644)

* Implemented compute_disk_create_secondary_custom sample, created test

* Fixed code

* Fixed variable

* Fixed code

* Fixed whitespace

* Fixed whitespace

* feat(compute): add compute snapshot schedule create/get/edit/list/delete samples (#9742)

* Implemented compute_snapshot_schedule_delete and compute_snapshot_schedule_create samples, created test

* Fixed test

* Added compute_snapshot_schedule_get sample, created test

* Fixed naming

* Implemented compute_snapshot_schedule_edit, created test

* Fixed naming

* Implemented compute_snapshot_schedule_list sample, created test

* Cleaned resources

* Cleaned resources

* Cleaned resources

* Cleaned resources

* Fixed test

* Added comment

* Fixed tests

* Fixed code

* Fixed code as requested in the comments

* feat(compute): add compute disk create with snapshot schedule (#9788)

* Implemented compute_disk_create_with_snapshot_schedule sample, created test

* Fixed code

* Fixed code

* Fixed test

* Fixed code

* Fixed code as requested in the comments

* Fixed lint issue

* Fixed lint issue

* Deleted redundant code

* feat(tpu): add tpu queued resources time bound sample. (#9617)

* Changed package, added information to CODEOWNERS

* Added information to CODEOWNERS

* Added timeout

* Fixed parameters for test

* Fixed DeleteTpuVm and naming

* Added comment, created Util class

* Fixed naming

* Fixed whitespace

* Split PR into smaller, deleted redundant code

* Implemented tpu_queued_resources_create, tpu_queued_resources_get, tpu_queued_resources_delete_force and tpu_queued_resources_delete samples, created tests

* Implemented tpu_queued_resources_time_bound sample, created test

* Changed zone for tpu

* Cleanup resources

* Fixed tests

* Fixed test

* Fixed code as requested in the comments

* Fixed code as requested in the comments

* fix(job): delete old region tag update_job_with_field_mask (#9940)

* feat(job): migrate region tags to include product prefix (#9966)

* fix(endpoints): migrate all regions (#9943)

* fix: disable flakybot reporting (#9968)

* chore(job): remove unused region tags (#9969)

* feat(securitycenter): Add Resource SCC Management API Org ETD Custom Module code samples (Update, Get Eff, List Eff, List Desc, Validate) (#9912)

* sample codes for event threat detection custom modules

* fixed lint

* addressed comments

* lint fix

* addressed comments

---------

Co-authored-by: OremGLG <[email protected]>
Co-authored-by: eapl.me <[email protected]>
Co-authored-by: Тетяна Ягодська <[email protected]>
Co-authored-by: Jennifer Davis <[email protected]>
Co-authored-by: lovenishs04 <[email protected]>
Co-authored-by: alarconesparza <[email protected]>
Co-authored-by: Jennifer Davis <[email protected]>
Co-authored-by: code-review-assist[bot] <182814678+code-review-assist[bot]@users.noreply.github.com>
Co-authored-by: Brian Dorsey <[email protected]>
@ludoch ludoch requested review from a team as code owners January 9, 2025 13:08
Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

Hi team,

I'm currently reviewing this pull request and will post my feedback in a few minutes. In the meantime, I'm providing this summary of the pull request's content for you and other reviewers to quickly get up to speed on the changes and intent.

This pull request, submitted by ludoch, aims to upgrade all Google App Engine Java 8 runtime samples to the latest Java 21 runtime. The Java 8 runtime is now end-of-life (EOL). The approach taken minimizes code changes; primarily, only the XML configuration files (like appengine-web.xml) are modified to reflect the Java 21 runtime and the compatible EE8 setting. The author has manually tested the changes locally using the devappserver and also deployed to a cloud project.

Here's a summary of the changes:

  • .github/CODEOWNERS: Added @GoogleCloudPlatform/cloud-ml-translate-dev to the codeowners for the /translate directory.
  • .kokoro/tests/run_prptst_tests.sh and .kokoro/tests/run_test_java.sh: Removed code related to Java 8 periodic builds and FlakyBot.
  • appengine-java21/README.md: Updated README to reflect Java 21 runtime and updated links.
  • Various appengine-java21 subdirectory files: Updated copyright years and appengine-web.xml files to use Java 21 runtime and EE8 settings. Specific changes include updated runtime tags in appengine-web.xml files and adjustments to pom.xml files to reflect the new artifact IDs.

Note: The pull request mentions dependency on internal changes (cl/691482072) for Google public pages. This is being coordinated with the Google Cloud documentation team.

The author has included a checklist in the description, indicating that tests and lint checks have passed (or are expected to pass).

I'll provide a more detailed review shortly.


Here's a little haiku to brighten your day:

Java eight is gone,
Twenty-one now takes its place,
Code runs smoothly on.

Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

This pull request migrates App Engine services from the deprecated Java 8 runtime to Java 21. This is a necessary change and the approach of minimizing code changes by leveraging the EE8 compatibility setting is sound. The changes appear thorough, addressing pom.xml files, appengine-web.xml, READMEs, and build scripts. However, some minor improvements and additional checks are recommended.

Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

Hi team,

I'm currently reviewing this pull request and will post my feedback in a few minutes. In the meantime, here's a summary of the changes for you and other reviewers to quickly get up to speed:

This pull request, submitted by ludoch, aims to upgrade all Google App Engine Java 8 runtime samples to the latest Java 21 runtime. The Java 8 runtime is now end-of-life (EOL). The upgrade leverages the compatible EE8 setting, meaning the servlet code itself should remain largely unchanged. The primary changes are confined to the XML configuration files, such as appengine-web.xml.

Here's a breakdown of the changes:

  • Updated appengine-web.xml files: Across multiple samples, the <runtime> tag has been updated from java8 to java21. Additionally, <app-engine-apis> is set to true, and a <system-properties> section is added with <property name="appengine.use.EE8" value="true"/> to ensure compatibility. (See numerous files for specific examples)
  • CODEOWNERS file modification: Line 82 was modified to add @GoogleCloudPlatform/cloud-ml-translate-dev to the codeowners for the /translate directory.
  • Removal of Java 8 specific Kokoro test logic: The run_prptst_tests.sh and run_test_java.sh scripts had sections related to Java 8 flaky bot reporting removed. This is because Java 8 is no longer supported.
  • README.md updates: Numerous README.md files were updated to reflect the Java 21 runtime and to correct links to the updated code locations.

The author reports that the changes have been manually tested using the local devappserver and deployed to a cloud project.

However, the pull request is currently blocked pending changes to Google's public pages (cl/691482072), which are being coordinated with the Google Cloud documentation team.

I'll provide a more detailed review shortly.


Here's a little haiku to brighten your day:

Java eight is gone,
Twenty-one now takes its place,
Code flows smoothly on.

Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

This pull request migrates the App Engine Java samples from Java 8 to Java 21. This is a necessary update due to Java 8's end-of-life. The approach of minimizing code changes by leveraging the EE8 compatibility setting is sensible. The changes appear thorough, addressing pom.xml files, appengine-web.xml, READMEs, and build scripts. However, some refinements are needed before merging.

Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

Hi team,

I'm currently reviewing this pull request and will post my feedback in a few minutes. In the meantime, here's a summary to help you and other reviewers quickly understand the changes:

This pull request aims to upgrade all Google App Engine Java 8 runtime samples to the latest Java 21 runtime, leveraging the compatible EE8 setting. The intent is to minimize code changes, focusing primarily on updating the XML configuration files (like appengine-web.xml) while preserving the existing servlet code.

Here's a breakdown of the changes:

  • .github/CODEOWNERS: Added @GoogleCloudPlatform/cloud-ml-translate-dev to the codeowners for the /translate directory.
  • .kokoro/tests/run_prptst_tests.sh and .kokoro/tests/run_test_java.sh: Removed code related to Java 8 periodic builds and FlakyBot.
  • appengine-java21/README.md: Updated README to reflect Java 21 runtime and updated links.
  • Various files within appengine-java21 subdirectories (analytics, appidentity, bigquery, bigtable, cloudsql, datastore-indexes-exploding, datastore-indexes-perfect, datastore-indexes, datastore-schedule-export, datastore, firebase-tictactoe, gaeinfo, guestbook-cloud-datastore, helloworld, iap, images, mail, mailgun, mailjet, memcache, metadata, oauth2, pubsub, requests, search, sendgrid, spanner, sparkjava-helloworld, static-files, taskqueues-deferred, taskqueues-pull, taskqueues-push, tasks/app, tasks/quickstart): These files have been updated to reflect the Java 21 runtime and EE8 settings in appengine-web.xml files. Specific changes include copyright year updates and minor code adjustments for compatibility. The changes are mostly confined to configuration files and copyright headers.

The author, ludoch, has manually tested the changes using the local devappserver and deployed to a cloud project. However, the PR is currently blocked pending changes to Google public pages (cl/691482072), which are being coordinated with the Google Cloud documentation team.

I'll provide a more detailed review shortly.

And now, a little haiku to brighten your day:

Java eight is gone,
Twenty-one now takes its place,
Code flows smoothly on.

Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

This PR updates the Java runtime from Java 8 to Java 21 for App Engine. The approach of using the EE8 compatibility setting is good, minimizing changes to the Java source code itself. The changes seem comprehensive, covering various samples and configurations. However, ensure thorough testing across all affected samples, especially after the Google public pages changes are completed.

One small suggestion is to consolidate the changes to the copyright year (2015-2018 to 2024) in a separate commit. This improves the readability of the main changes in this PR and makes it easier to revert or cherry-pick the copyright update if needed.

Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

Hi team,

I'm currently reviewing this pull request and will post my feedback in a few minutes. In the meantime, here's a summary of the changes for you and other reviewers to quickly get up to speed:

This pull request, submitted by ludoch, aims to upgrade all Google App Engine Java 8 runtime samples to the latest Java 21 runtime. The Java 8 runtime is now end-of-life (EOL). The approach taken minimizes code changes; primarily, only the XML configuration files (like appengine-web.xml) are modified to use the compatible EE8 setting. The servlet code itself remains largely unchanged.

The author, ludoch, has manually tested the changes using the local devappserver and deployed to their cloud project. However, they are coordinating with the Google Cloud documentation team to make corresponding changes to the public Google pages, pending an internal change request (cl/691482072).

Here's a breakdown of the changes:

  • .github/CODEOWNERS: Added @GoogleCloudPlatform/cloud-ml-translate-dev to the codeowners for the /translate directory.
  • .kokoro/tests/run_prptst_tests.sh and .kokoro/tests/run_test_java.sh: Removed code related to Java 8 periodic builds and FlakyBot.
  • appengine-java21/README.md: Updated README to reflect Java 21 runtime and updated links.
  • Numerous other files across various directories (appengine-java21/analytics, appengine-java21/appidentity, appengine-java21/bigquery, appengine-java21/bigtable, appengine-java21/cloudsql, appengine-java21/datastore-indexes-exploding, appengine-java21/datastore-indexes-perfect, appengine-java21/datastore-indexes, appengine-java21/datastore-schedule-export, appengine-java21/datastore, appengine-java21/firebase-tictactoe, appengine-java21/gaeinfo, appengine-java21/guestbook-cloud-datastore, appengine-java21/helloworld, appengine-java21/iap, appengine-java21/images, appengine-java21/mail, appengine-java21/mailgun, appengine-java21/mailjet, appengine-java21/memcache, appengine-java21/metadata, appengine-java21/oauth2, appengine-java21/pubsub, appengine-java21/requests, appengine-java21/search, appengine-java21/sendgrid, appengine-java21/spanner, appengine-java21/sparkjava-helloworld, appengine-java21/static-files, appengine-java21/taskqueues-deferred, appengine-java21/taskqueues-pull, appengine-java21/taskqueues-push, appengine-java21/tasks/app, appengine-java21/tasks/quickstart): Updated copyright years and appengine-web.xml files to reflect Java 21 runtime and EE8 compatibility. Specific changes include updating artifact IDs, runtime versions, and adding <app-engine-apis>true</app-engine-apis> and <property name="appengine.use.EE8" value="true"/> to appengine-web.xml files.

Checklist: The author has provided a checklist, but its completion status is not yet verifiable.

I'll provide a more detailed review shortly.

Meanwhile, a little poem for your enjoyment:

Java eight is gone,
Java twenty-one's the way,
Code now runs so fast!

Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

This pull request migrates the Java runtime from Java 8 to Java 21 for Google App Engine Standard. The approach of using the EE8 compatibility setting is good, as it minimizes changes to the Java source code. The changes appear comprehensive, covering pom.xml files, appengine-web.xml, READMEs, and build scripts. Thorough testing is crucial to ensure compatibility with the Java 21 runtime. Please complete the checklist items before merging.

Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

Hi team,

I'm currently reviewing this pull request and will post my feedback in a few minutes. In the meantime, here's a summary of the changes for you and other reviewers to quickly get up to speed:

This pull request, submitted by ludoch, aims to upgrade all Google App Engine Java 8 runtime samples to the latest Java 21 runtime. The Java 8 runtime is now end-of-life (EOL). The upgrade leverages the compatible EE8 setting, minimizing code changes to only the XML configuration files (like appengine-web.xml). The author, ludoch, has manually tested the changes locally using the devappserver and also deployed them to a cloud project.

Key Changes:

  • The pull request modifies the following files:
    • .github/CODEOWNERS: Added @GoogleCloudPlatform/cloud-ml-translate-dev to the codeowners for the /translate directory.
    • .kokoro/tests/run_prptst_tests.sh and .kokoro/tests/run_test_java.sh: Removed Java 8-specific logic from the FlakyBot integration.
    • appengine-java21/README.md: Updated README to reflect the Java 21 runtime and corrected a link.
    • Numerous other files under the appengine-java21 directory: Updated copyright years and appengine-web.xml files to use java21 runtime and appengine.use.EE8 system property for EE8 compatibility. Specific changes include updating artifact IDs to reflect the Java 21 version and updating README files to point to the correct directories.

Status:

The pull request is pending approval and requires coordination with the Google Cloud documentation team for updates to their public pages (cl/691482072).

Checklist:

The author has provided a checklist which includes:

  • I have followed Sample Format Guide
  • pom.xml parent set to latest shared-configuration
  • Appropriate changes to README are included in PR
  • These samples need a new API enabled in testing projects to pass (let us know which ones)
  • These samples need a new/updated env vars in testing projects set to pass (let us know which ones)
  • Tests pass: mvn clean verify required
  • Lint passes: mvn -P lint checkstyle:check required
  • Static Analysis: mvn -P lint clean compile pmd:cpd-check spotbugs:check advisory only
  • This sample adds a new sample directory, and I updated the CODEOWNERS file with the codeowners for this sample
  • This sample adds a new Product API, and I updated the Blunderbuss issue/PR auto-assigner with the codeowners for this sample
  • Please merge this PR for me once it is approved

Let me know if you have any questions.

And now, a little poem to brighten your day:

Java eight is gone,
Java twenty-one's the way,
Code flows fresh and new.

Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

This pull request migrates the App Engine Java samples from Java 8 to Java 21. This is a necessary update due to Java 8's end-of-life. The approach of minimizing code changes by leveraging the EE8 compatibility setting is sound. The changes appear thorough, addressing pom.xml files, appengine-web.xml, READMEs, and build scripts. However, several areas need attention before merging.

@ludoch
Copy link
Contributor Author

ludoch commented Jan 9, 2025

@iennae I did the date changes, but now I see this in the header-check check:

"appengine-java21/bigtable/settings.gradle should have a copyright year of 2025
compute/cloud-client/src/main/java/compute/CreateInstanceWithRegionalDiskFromSnapshot.java should have a copyright year of 2025
compute/cloud-client/src/main/java/compute/disks/AttachRegionalDiskForce.java should have a copyright year of 2025
"
etc.

What is the rule there? It seems I am wasting a lot of time on these date changes that should be handled separately by a different team, globally to this repo.

PTAL anyway, to see if appart nits, you are ok with this PR. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants