Skip to content

Conversation

@eleonoradgr
Copy link
Contributor

@eleonoradgr eleonoradgr commented Feb 2, 2026

What changed?
we don't return an error when we heartbeat and the local passthrough is enabled for a namespace. It is part of heartbeat logic for #6862

Why?
Changing the onboarding from any state of onboarding to local passthrough is considered a legitimate operation, and the executor client handle it. Fixing the code to not return an error, the error is handled as unexpected behavior by the client, in this case we only want to use the expected behavior of stopping heartbeat.

How did you test it?
Changed the already existing unit tests and runnning:
go test github.com/uber/cadence/service/matching/tasklist

Potential risks
this endpoint is only used by the executor client, which can already handle this case as non error.

Release notes

Documentation Changes


Reviewer Validation

PR Description Quality (check these before reviewing code):

  • "What changed" provides a clear 1-2 line summary
    • Project Issue is linked
  • "Why" explains the full motivation with sufficient context
  • Testing is documented:
    • Unit test commands are included (with exact go test invocation)
    • Integration test setup/commands included (if integration tests were run)
    • Canary testing details included (if canary was mentioned)
  • Potential risks section is thoughtfully filled out (or legitimately N/A)
  • Release notes included if this completes a user-facing feature
  • Documentation needs are addressed (or noted if uncertain)

@gitar-bot
Copy link

gitar-bot bot commented Feb 2, 2026

🔍 CI failure analysis for db55718: The CI validation continues to fail because the PR title still uses square brackets and now has an unwanted space. The correct format requires parentheses with no space between the type and scope.

Issue

The CI validation for "Validate PR title follows conventional commit format" continues to fail (latest job: 62179890442).

Root Cause

The PR title was updated but still does not conform to the conventional commit specification. The current title is:

fix [shard-distributor]: remove error for local passthrough

Previous Failures (jobs 62179543862, 62179720928)

The original title fix[shard-distributor]: used square brackets instead of parentheses.

Current Failure (job 62179890442)

The title fix [shard-distributor]: now has two problems:

  1. Still uses square brackets [] instead of parentheses ()
  2. Has an unwanted space between fix and [shard-distributor]

According to the Conventional Commits specification, the correct format is:

<type>(<scope>): <description>

The validator's pattern matching fails because:

  • It expects no space between the type and scope: fix(...):
  • It expects parentheses, not square brackets

Details

The error from job 62179890442:

No release type found in pull request title "fix [shard-distributor]: remove error for local passthrough". 
Add a prefix to indicate what kind of release this pull request corresponds to.

The validator cannot recognize "fix" as the release type due to these formatting issues.

Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Clean behavioral change that correctly handles local passthrough mode during heartbeat. The previous minor finding about the log message remains valid - the message "no calls to heartbeat should be allowed" slightly contradicts the graceful handling.

💡 Quality: Log message contradicts new behavior

📄 service/sharddistributor/handler/executor.go:66

The log message says "no calls to heartbeat should be allowed" but the code now successfully handles this case and returns a valid response. This is misleading for anyone reading the logs.

Consider updating the message to better reflect the actual behavior, for example:

  • "Migration mode is local passthrough, returning mode without shard assignments"
  • "Heartbeat received during local passthrough mode, returning current state"

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@eleonoradgr eleonoradgr changed the title fix[shard-distributor]: remove error for local passthrough fix [shard-distributor]: remove error for local passthrough Feb 2, 2026
@eleonoradgr eleonoradgr changed the title fix [shard-distributor]: remove error for local passthrough fix: [shard-distributor] remove error for local passthrough Feb 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant