-
Notifications
You must be signed in to change notification settings - Fork 18
Update Webhook Triggers: Add Async and Sync #737
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
base: main
Are you sure you want to change the base?
Conversation
This PR updates the webhook triggers documentation with async and synchronous modes. This also updates the Webhook Ttigger image.
| * The workflow may take longer to run, but the caller does not need the result | ||
|
|
||
|
|
||
| **When the response is sent:** |
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.
maybe add ? at the end
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 put this down as a statement more than a question Tuch. Do you think it'll make more sense as a question?
|
|
||
| Immediately, during the same HTTP request that triggered the workflow. | ||
|
|
||
| **What this response represents:** |
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.
add ? at the end
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 put this down as a statement more than a question Tuch. Do you think it'll make more sense as a question?
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.
Ooh okay 🤔 , If the explanation is in a newline then it looks like a question but if it's oneline it's fine as is
**What this response represents:** It confirms that OpenFn has accepted the webhook and scheduled the workflow to run. It does **not** include the workflow’s output.
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.
@theroinaochieng this looks good just tiny suggestion on questions like
when the response is sent: should end with ?
taylordowns2000
left a comment
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.
Hey @theroinaochieng , this is backwards. Async is the default mode (see app UI) and that means it replies immediately. Sync is the new, non-default option which will execute the workflow synchronously and reply when finished.
This PR updates the webhook triggers documentation with async and synchronous modes.
This also updates the Webhook Trigger image.
Short Description
A one or two-sentence description of what this PR does.
Closes #<#736>
Details
Add any more details that may be relevant to the reviewer. How did you approach
these docs? Are there any parts you struggled with, or that need particularly
careful review?
AI Usage
Please disclose how you've used AI in this work (it's cool, we just want to
know!):
You can read more details in our
Responsible AI Policy