Skip to content

chore(shard-manager): provide metadata in constructor#7711

Merged
dkrotx merged 1 commit intocadence-workflow:masterfrom
dkrotx:dkrot/shard-processor-creation-refactoring
Feb 16, 2026
Merged

chore(shard-manager): provide metadata in constructor#7711
dkrotx merged 1 commit intocadence-workflow:masterfrom
dkrotx:dkrot/shard-processor-creation-refactoring

Conversation

@dkrotx
Copy link
Member

@dkrotx dkrotx commented Feb 16, 2026

What changed?
Refactoring for #7712:
Providing shard executor metadata to the constructor instead of Initalising this separately.
In addition, unifying panic -> logger.Fatal as we do in other cases.

Why?

+Better readability as code is more aligned with other usages of log.Fatal.
Passing metadata to constructor allows to avoid partial structure initialization.

How did you test it?
go test -v ./service/matching/handler

Potential risks

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)

dkrotx added a commit to dkrotx/cadence that referenced this pull request Feb 16, 2026
During refactoring for cadence-workflow#7711
When shard was lost and matcher is on shard-manager, we returned untyped
error which could potentially lead to wrong retries from client.
Also, returning right error ensures we have the same stats/logs as we
would have without shard-manager.

Signed-off-by: Jan Kisel <dkrot@uber.com>
Ininitalising this separately is a bit unusual.
In addition, unifying panic -> logger.Fatal as we do in other cases.

Signed-off-by: Jan Kisel <dkrot@uber.com>
@dkrotx dkrotx force-pushed the dkrot/shard-processor-creation-refactoring branch from 6bbd54b to c9c0f09 Compare February 16, 2026 17:21
@gitar-bot
Copy link

gitar-bot bot commented Feb 16, 2026

Code Review ✅ Approved

Clean refactoring that moves metadata into the executor constructor and unifies error handling from panic to logger.Fatal. The reorder in GetShardProcess is semantically equivalent. No issues found.

Rules ✅ All requirements met

Repository Rules

PR Description Quality Standards: PR description meets all quality standards. [What changed?] has clear summary with issue #7712 linked. [Why?] explains technical rationale including 'Passing metadata to constructor allows to avoid partial structure initialization' and alignment with other log.Fatal usages. [How did you test it?] provides exact command: go test -v ./service/matching/handler.
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

dkrotx added a commit to dkrotx/cadence that referenced this pull request Feb 16, 2026
During refactoring for cadence-workflow#7711
When shard was lost and matcher is on shard-manager, we returned untyped
error which could potentially lead to wrong retries from client.
Also, returning right error ensures we have the same stats/logs as we
would have without shard-manager.

Signed-off-by: Jan Kisel <dkrot@uber.com>
@dkrotx dkrotx merged commit 29c6b41 into cadence-workflow:master Feb 16, 2026
42 checks passed
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.

2 participants