Skip to content

Conversation

timl3136
Copy link
Member

@timl3136 timl3136 commented Jun 5, 2025

Detailed Description
[In-depth description of the changes made to the interfaces, specifying new fields, removed fields, or modified data structures]
Introduce CronOverlapPolicy in StartWorkflowOptions from cadence-idl.
Also create empty methods for DeleteDomain operation matching update from cadence-idl.

Impact Analysis

  • Backward Compatibility: [Analysis of backward compatibility]
  • Backward compatible since this is a new field added
  • Forward Compatibility: [Analysis of forward compatibility]
  • Forward compatible as this field will be default to not affect any current behavior.

Testing Plan

  • Unit Tests: [Do we have unit test covering the change?]
  • Yes
  • Persistence Tests: [If the change is related to a data type which is persisted, do we have persistence tests covering the change?]
  • Persistence tests will be implemented in server repo
  • Integration Tests: [Do we have integration test covering the change?]
  • will be implemented in server repo
  • Compatibility Tests: [Have we done tests to test the backward and forward compatibility?]
  • No yet

Rollout Plan

  • What is the rollout plan?
  • Rollout client change after server one landed
  • Does the order of deployment matter?
  • Not really since server will drop the field if not recognized and default value won't affect current behavior
  • Is it safe to rollback? Does the order of rollback matter?
  • Yes, order does not matter
  • Is there a kill switch to mitigate the impact immediately?
  • Rollback if needed

Copy link
Member

Choose a reason for hiding this comment

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

we need to update the corresponding types/mapper functions in common/types/shared.go.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I plan to land the corresponding change in server first before releasing this.

Copy link

codecov bot commented Jun 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.20%. Comparing base (291e3dd) to head (d87e376).
Report is 1 commits behind head on master.

Files with missing lines Coverage Δ
internal/client.go 75.86% <ø> (ø)
internal/common/auth/service_wrapper.go 93.81% <100.00%> (+0.13%) ⬆️
internal/common/convert.go 100.00% <100.00%> (ø)
internal/common/isolationgroup/service_wrapper.go 91.84% <100.00%> (+0.13%) ⬆️
internal/common/metrics/service_wrapper.go 98.40% <100.00%> (+0.03%) ⬆️
internal/internal_workflow_client.go 89.39% <100.00%> (+0.02%) ⬆️

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 291e3dd...d87e376. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@taylanisikdemir taylanisikdemir left a comment

Choose a reason for hiding this comment

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

Did we test this e2e with and without setting the policy?

@timl3136
Copy link
Member Author

Did we test this e2e with and without setting the policy?

I have tested this locally, I plan to add an integration test later.

@timl3136 timl3136 merged commit 6f6a1fa into cadence-workflow:master Jun 26, 2025
11 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.

3 participants