Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Fix WAIT task is not working properly (after v3.13.0) #3639

Merged
merged 6 commits into from
Jun 19, 2023

Conversation

yohanyflores
Copy link
Contributor

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes (Please run ./gradlew generateLock saveLock to refresh dependencies)
  • WHOSUSING.md
  • Other (please describe):

NOTE: Please remember to run ./gradlew spotlessApply to fix any format violations.

Changes in this PR

Issue #3402

Alternatives considered

? taskModel.getWaitTimeout() + 1
: workflowOffsetTimeout;
if (taskModel.getTaskType().equals(TaskType.TASK_TYPE_WAIT)) {
long deltaInSeconds =
Copy link
Contributor

Choose a reason for hiding this comment

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

what happens if the wait task has no timeout? This can happen if the WAIT is configured to wait for an external trigger without a timeout value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't take that case into account because it didn't seem to me that the Wait class allowed it, as it requires the use of duration or until. The only way to have a zero timeout is by setting the until parameter to the start of the epoch, January 1, 1970, at 0 hours.

However, I imagine it might be possible in another way that I'm not aware of.

Would you like me to share a code snippet that takes that case into account?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how to update the code. Should I create another Pull Request?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also updated the branch in the same way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @yohanyflores , I see that when deltaInSeconds <= 0 shouldn't we mark the postponeDuration to 0 instead of 1 because there is no point in waiting for 1 more second?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@manan164 Thanks for the change you suggest, you are right!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@manan164 I updated the branch with the changes and tests.

@v1r3n v1r3n merged commit 3920aee into Netflix:main Jun 19, 2023
mabelellerker pushed a commit to alfasoftware/conductor that referenced this pull request Aug 4, 2023
…g-properly

Fix WAIT task is not working properly (after v3.13.0)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants