S226 local dev fix by jlorper · Pull Request #97 · Worklytics/appengine-pipelines

jlorper · 2026-05-27T18:31:19Z

Fixes

Attempt to fix local dev datastore after Google bom update to 26.83.0 that introduced some changes to the datastore that broken local dev (that get's injected from worklyytics' main project)

Change implications

breaking change to API? no
changes dependencies? no

S219 : prep next version

…es/security/dependabot/9 (#95)

claude

⚠️ Code review skipped — your organization's overage spend limit has been reached.

Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.

Once credits are available, reopen this pull request to trigger a review.

Copilot

Pull request overview

This PR aims to restore correct local-development behavior when using the Datastore emulator after a Google Cloud Java BOM update, by avoiding DatastoreOptions.toBuilder() (which can drop the emulator host) and instead rebuilding options via DatastoreOptions.newBuilder() while explicitly preserving host.

Changes:

Replaced DatastoreOptions.toBuilder() / getDefaultInstance().toBuilder() usage with explicit newBuilder() copies that retain host.
Removed a couple of “round-trip rebuilds” of DatastoreOptions and passed options/services through directly.
Bumped the fork version and documented the change (plus added internal agent docs).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
java/src/main/java/com/google/appengine/tools/mapreduce/ShardedJobAbstractSettings.java	Rebuilds `DatastoreOptions` from default instance via `newBuilder()` while preserving `host`.
java/src/main/java/com/google/appengine/tools/mapreduce/MapReduceJob.java	Updates multiple stage-specific Datastore construction sites to avoid `toBuilder()` and preserve `host`.
java/src/main/java/com/google/appengine/tools/mapreduce/impl/util/RequestUtils.java	Rebuilds request-derived `DatastoreOptions` without `toBuilder()` to retain emulator `host`.
java/src/main/java/com/google/appengine/tools/mapreduce/impl/shardedjob/pipeline/FinalizeShardsInfos.java	Stops rebuilding options via `toBuilder()` and uses `datastoreOptions.getService()`.
java/src/main/java/com/google/appengine/tools/mapreduce/impl/shardedjob/pipeline/DeleteShardedJob.java	Stops passing a `toBuilder().build()` copy of `DatastoreOptions`.
java/pom.xml	Version bump to `0.3+worklytics.14`.
java/changes.md	Adds changelog entry describing the `toBuilder()` emulator-host issue and affected files.
.agents/plans/FIX-DATASTORE-TOBUILDER.md	Internal plan documenting root cause and the intended replacement pattern.
.agents/CLAUDE.md	Internal repository guidance for Claude Code usage and project architecture/testing notes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+        DatastoreOptions defaultInstance = DatastoreOptions.getDefaultInstance();
+        DatastoreOptions.Builder b = DatastoreOptions.newBuilder()
+            .setProjectId(defaultInstance.getProjectId())
+            .setCredentials(defaultInstance.getCredentials())
+            .setTransportOptions(defaultInstance.getTransportOptions());
+        java.util.Optional.ofNullable(defaultInstance.getHost()).ifPresent(b::setHost);
+        java.util.Optional.ofNullable(settings.getNamespace()).ifPresent(b::setNamespace);
+        datastore = b.build().getService();


+        DatastoreOptions defaultInstance = DatastoreOptions.getDefaultInstance();
+        DatastoreOptions.Builder b = DatastoreOptions.newBuilder()
+            .setProjectId(defaultInstance.getProjectId())
+            .setCredentials(defaultInstance.getCredentials())
+            .setTransportOptions(defaultInstance.getTransportOptions());
+        java.util.Optional.ofNullable(defaultInstance.getHost()).ifPresent(b::setHost);


+        DatastoreOptions defaultInstance = DatastoreOptions.getDefaultInstance();
+        DatastoreOptions.Builder b = DatastoreOptions.newBuilder()
+            .setProjectId(defaultInstance.getProjectId())
+            .setCredentials(defaultInstance.getCredentials())
+            .setTransportOptions(defaultInstance.getTransportOptions());
+        java.util.Optional.ofNullable(defaultInstance.getHost()).ifPresent(b::setHost);


  protected Job<?> createShardsJob(int start, int end) {
-    return new DeleteShardsInfos(datastoreOptions.toBuilder().build(), getJobId(), start, end);
+    return new DeleteShardsInfos(datastoreOptions, getJobId(), start, end);
  }


  public Value<Void> run() {
-    Datastore datastore = datastoreOptions.toBuilder().build().getService();
+    Datastore datastore = datastoreOptions.getService();



+   (Google Cloud Java BOM ≥ 26.83.0). Replaced all `toBuilder()` / `getDefaultInstance().toBuilder()`
+   calls with explicit `DatastoreOptions.newBuilder()` copies that preserve the `host` field.


    DatastoreOptions defaultInstance = DatastoreOptions.getDefaultInstance();

-    DatastoreOptions.Builder builder = defaultInstance.toBuilder();
+    DatastoreOptions.Builder builder = DatastoreOptions.newBuilder()
+        .setProjectId(defaultInstance.getProjectId())
+        .setCredentials(defaultInstance.getCredentials())
+        .setTransportOptions(defaultInstance.getTransportOptions());
+    Optional.ofNullable(defaultInstance.getHost()).ifPresent(builder::setHost);


  @Override
  public Value<Void> run() {
-    Datastore datastore = datastoreOptions.toBuilder().build().getService();
+    Datastore datastore = EnvironmentUtils.datastoreBuilderFromDatastoreOptions(datastoreOptions).build().getService();


  public Value<Void> run() {
-    Datastore datastore = datastoreOptions.toBuilder().build().getService();
+    // if coming from deserialization may lose transient properties that cause NPE
+    Datastore datastore = EnvironmentUtils.datastoreBuilderFromDatastoreOptions(datastoreOptions).build().getService();


…t order Test was written for old order (enqueue tasks then commit Datastore). Current implementation commits Datastore first, so when Datastore fails tasks are never enqueued and nothing needs to be deleted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

# Conflicts: # java/pom.xml

finalize() is deprecated and marked for removal in modern Java. The override only logged a warning for unclosed transactions — no actual resource cleanup. Callers already have rollbackIfActive() for explicit cleanup. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…local-dev-fix # Conflicts: # java/pom.xml # java/src/test/java/com/google/appengine/tools/txn/PipelineBackendTransactionImplTest.java

+    Set<PipelineTaskQueue.TaskReference> taskReferences = Collections
+        .singleton(PipelineTaskQueue.TaskReference.of("queue1", "task-ref"));


  // Only used in tests
  public AppEngineBackEnd(Options options, PipelineTaskQueue taskQueue, AppEngineServicesService appEngineServicesService) {
-    this(options.getDatastoreOptions().toBuilder().build().getService(), taskQueue, appEngineServicesService);
+    this(EnvironmentUtils.datastoreBuilderFromDatastoreOptions(options.getDatastoreOptions()).build().getService(), taskQueue, appEngineServicesService);


eschultink and others added 9 commits February 25, 2026 06:04

bump version number; delete old pom

7e46e5e

bump to java21

c4a0933

cleanup workflow

44771b0

organize dependencies

8b20580

Merge pull request #93 from Worklytics/s219-prep-next-version

a6375f0

S219 : prep next version

bump jackson to solve https://github.com/Worklytics/appengine-pipelin…

5125223

…es/security/dependabot/9 (#95)

Init claude on project

7584c38

Plan to fix DatastoreOptions.toBuilder() dropping host

29cc5ae

Apply fix, update version to 0.3+worklytics.14

b3e5c52

claude Bot reviewed May 27, 2026

View reviewed changes

jlorper requested review from aperez-worklytics, Copilot and eschultink May 27, 2026 18:31

jlorper self-assigned this May 27, 2026

Copilot started reviewing on behalf of jlorper May 27, 2026 18:31 View session

parkcoll approved these changes May 27, 2026

View reviewed changes

Copilot AI reviewed May 27, 2026

View reviewed changes

jlorper added 7 commits May 27, 2026 21:26

NoCredentials on tests

a2bb7e1

Simplify DatastoreOptions usage. Bound jackson version to 2.9+

0cff9ea

Use NoCredentials on tests / local

8ba00b8

Helper method to build DatastoreOptions.Builder from default

a02db65

Utils class to unify local/test checks handling.

1de653d

Do not set credentials upfront, as if null provokes NPE

db2c790

Deal with datastoreOptions coming from serialization cases

913f653

github-code-quality Bot found potential problems May 28, 2026

View reviewed changes

jlorper added 4 commits May 28, 2026 12:23

Reuse DatastoreOptions.Builder from settings

bae8dba

Split regex patterns for mapreduce impl packages tests

5e719f6

Fix CI setting to use matrix

7bed34b

Fix regex patterns

541dabc

davidfq approved these changes May 28, 2026

View reviewed changes

jlorper and others added 6 commits May 28, 2026 13:11

pass host property for datastore tests

71858da

Merge remote-tracking branch 'origin/main' into s226-local-dev-fix

ff4cfe5

# Conflicts: # java/pom.xml

Compile to Java 21 bytecode

775955a

Merge remote-tracking branch 'refs/remotes/origin/rc-0.13' into s226-…

8b52a84

…local-dev-fix # Conflicts: # java/pom.xml # java/src/test/java/com/google/appengine/tools/txn/PipelineBackendTransactionImplTest.java

github-code-quality Bot found potential problems May 28, 2026

View reviewed changes

Comment thread java/src/test/java/com/google/appengine/tools/txn/PipelineBackendTransactionImplTest.java

Comment on lines +51 to +52

Set<PipelineTaskQueue.TaskReference> taskReferences = Collections

.singleton(PipelineTaskQueue.TaskReference.of("queue1", "task-ref"));

Fix tests

52d210f

github-code-quality Bot found potential problems May 28, 2026

View reviewed changes

jlorper enabled auto-merge May 28, 2026 16:32

jlorper force-pushed the s226-local-dev-fix branch from 5c39231 to f6d8049 Compare May 28, 2026 17:28

Better naming for test matrix entries

27dd669

jlorper force-pushed the s226-local-dev-fix branch from f6d8049 to 27dd669 Compare May 28, 2026 17:32

jlorper disabled auto-merge May 28, 2026 19:25

jlorper merged commit 80745da into main May 28, 2026
15 of 27 checks passed

jlorper deleted the s226-local-dev-fix branch May 28, 2026 19:25

jlorper mentioned this pull request May 29, 2026

s226 - if emulator is present consider it as not cloud environment #98

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

S226 local dev fix#97

S226 local dev fix#97
jlorper merged 28 commits into
mainfrom
s226-local-dev-fix

jlorper commented May 27, 2026

Uh oh!

claude Bot left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		(Google Cloud Java BOM ≥ 26.83.0). Replaced all `toBuilder()` / `getDefaultInstance().toBuilder()`
		calls with explicit `DatastoreOptions.newBuilder()` copies that preserve the `host` field.

		Set<PipelineTaskQueue.TaskReference> taskReferences = Collections
		.singleton(PipelineTaskQueue.TaskReference.of("queue1", "task-ref"));

Conversation

jlorper commented May 27, 2026

Fixes

Change implications

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants