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

Refactoring: extract GetNextScheduledTime into a separate function #6817

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

Conversation

ychebotarev
Copy link
Contributor

@ychebotarev ychebotarev commented Nov 15, 2024

What changed?

Extract GetNextScheduledTime into a separate function.

Why?

  1. To be able to re-use it.
  2. To be able to provide custom time for RegenerateActivityRetryTask.

How did you test it?

run tests

Potential risks

none

Is hotfix candidate?

no

@@ -169,3 +169,27 @@ func GetPendingActivityInfo(

return p, nil
}

func GetNextScheduleTime(ai *persistence.ActivityInfo) time.Time {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
func GetNextScheduleTime(ai *persistence.ActivityInfo) time.Time {
func GetNextScheduledTime(ai *persistence.ActivityInfo) time.Time {

pleease...

// * in this case we should use current schedule time
// * this is a retry
// * next scheduled time will be calculated, based on the retry policy and last time when activity was completed
// * note - if delay interval was provided in the response it will be ignored
Copy link
Member

Choose a reason for hiding this comment

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

note is not relevant here.

@@ -173,7 +173,8 @@ func updateActivityOptions(
// eventually matching service will call history service (recordActivityTaskStarted)
// history service will return error based on stamp. Task will be dropped

err = mutableState.RegenerateActivityRetryTask(ai)
nextScheduleTime := workflow.GetNextScheduleTime(ai)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
nextScheduleTime := workflow.GetNextScheduleTime(ai)
nextScheduledTime := workflow.GetNextScheduledTime(ai)

@alexshtin alexshtin changed the title refactoring - extract GetNextScheduleTime into a separate function Refactoring: extract GetNextScheduledTime into a separate function Nov 15, 2024
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