-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Enhance WAIT task to support time-based wait scenarios #2986
Conversation
String until = | ||
Optional.ofNullable(task.getInputData().get(UNTIL_INPUT)).orElse("").toString(); | ||
|
||
if (Strings.isNotBlank(duration) && Strings.isNotBlank(until)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strings
is from log4j
. Please use StringUtils
from commons-lang3
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated.
if (Strings.isNotBlank(duration) && Strings.isNotBlank(until)) { | ||
task.setReasonForIncompletion( | ||
"Both 'duration' and 'until' specified. Please provide only one input"); | ||
task.setStatus(FAILED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the status be FAILED_WITH_TERMINAL_ERROR
? This error is not recoverable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
String until = | ||
Optional.ofNullable(task.getInputData().get(UNTIL_INPUT)).orElse("").toString(); | ||
|
||
if (Strings.isNotBlank(duration) && Strings.isNotBlank(until)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Task validations are typically done when the workflow is registered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added this as part of workflow creation.
} else if (Strings.isNotBlank(until)) { | ||
try { | ||
Date expiryDate = parseDate(until); | ||
System.out.println("expiryDate: " + expiryDate); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove SOP?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
} catch (ParseException parseException) { | ||
task.setReasonForIncompletion( | ||
"Invalid/Unsupported Wait Until format. Provided: " + until); | ||
task.setStatus(FAILED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could also be FAILED_WITH_TERMINAL_ERROR
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
if (Strings.isNotBlank(duration)) { | ||
|
||
Duration timeDuration = parseDuration(duration); | ||
task.getOutputData() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of adding the timeout
to the output data, we suggest that its added as a field to the TaskModel
. We think its a better fit because,
TaskModel
is internal only and fields in it are not exposed to the user. Adding it to outputData makes thetimeout
field part of the task contract.- We have plans to create custom TaskModels which only contain fields for a particular task type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated to use TaskModel
task.getOutputData() | ||
.put(TIMEOUT, System.currentTimeMillis() + (timeDuration.getSeconds() * 1000)); | ||
long seconds = timeDuration.getSeconds(); | ||
task.setCallbackAfterSeconds(seconds); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The callbackAfterSeconds
is moot since WAIT
is a synchronous task and is not added to a queue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated and removed.
@v1r3n can you update the documentation section for |
1. Added validations at the time of workflow creation 2. Using commons lang StringUtils 3. Using TaskModel to store timeout 4. Update documentation
polyglot-clients/README.md
Outdated
# SDKs for other languages | ||
|
||
Language specific client SDKs are maintained at a dedicated [conductor-sdk](https://github.com/conductor-sdk) repository. | ||
All the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incomplete?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
public Wait() { | ||
super(TASK_TYPE_WAIT); | ||
} | ||
|
||
public static Duration parseDuration(String text) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider moving these static methods to Utils?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations under the License. | ||
*/ | ||
package com.netflix.conductor.utils; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is already a Utils package here. Can you move this class under the same package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
* Update Wait.java * Add support for waiting for time duration or until a specific date/time * add unit tests and SDK methods * formatting * Remove Guava dependency * Fix formatting * Wait task updates 1. Added validations at the time of workflow creation 2. Using commons lang StringUtils 3. Using TaskModel to store timeout 4. Update documentation * remove callbackafter * add the break * Fix readme * Move the date parsing to a util class * move DateTimeUtils to the common utils package
Pull Request type
./gradlew generateLock saveLock
to refresh dependencies)Changes in this PR
Enhance wait task to be able to wait for a specific amount of time.
One commonly asked use case is to allow a task to wait for specific duration of time (e.g. 15 minutes) or until certain date/time (Dec 31, 2022).
These updates allows users to configure Wait tasks with two inputs:
Wait For time duration
Format duration as "XhYmZs"
Wait until specific date/time
Specify the date as one of the formats: