-
Notifications
You must be signed in to change notification settings - Fork 54
[Durable] Custom orchestration status #592
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
Conversation
99e0e4d
to
e1819ac
Compare
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.
One minor comment and one question. Other than that, LGTM.
{ | ||
[Parameter( | ||
Mandatory = true, | ||
Position = 0, |
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.
Do we need to assign a Position
for this parameter? I took a look at the other Durable cmdlets and this is the only one with a Positional parameter.
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'm trying to be very careful with adding positional parameters in general, but in this case I believe this is justified. The cmdlet takes just one parameter, and the purpose of this parameter is very obvious, so I want to allow users omit the parameter name: Set-DurableCustomStatus "my status"
.
I think we can carefully allow some positional parameters for some other durable-related cmdlets, but we can do this later.
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.
Sounds good @AnatoliB, thank you.
[Parameter( | ||
Mandatory = true, | ||
Position = 0, | ||
ValueFromPipeline = true)] |
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.
What would the use case be here for ValueFromPipeline
? Something like:
# 1)
'Processing Tokyo' | Set-DurableCustomStatus
# 2)
@{ ProgressMessage = 'Processing Seattle'; Stage = 2 } | Set-DurableCustomStatus
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.
Yes, exactly.
Feature documentation: Custom orchestration status.
Usage sample: