-
Notifications
You must be signed in to change notification settings - Fork 3
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
INN-2306: Implement step.waitForEvent #26
Conversation
466fa53
to
967edb7
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.
Looks good, we can address the return in follow-ups along with the others.
if (stepResult != null) { | ||
return stepResult | ||
} | ||
return null // TODO should this throw an exception? |
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.
It can also return EventPayload
as well i think.
https://www.inngest.com/docs/reference/functions/step-wait-for-event#step-wait-for-event-id-options-promise-null-event-payload
https://github.com/inngest/inngest-py/blob/4547a366bdab61b6fffab04db4d6c85a7191ea98/inngest/_internal/step_lib/step_async.py#L387-L393
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.
adding a TODO
https://github.com/inngest/inngest/blob/main/docs/SDK_SPEC.md#533-wait-for-event https://www.inngest.com/docs/reference/functions/step-wait-for-event#step-wait-for-event-id-options-promise-null-event-payload Currently only supporting the `if` optional condition, will add `match` later (`match` is still sent over the wire as an `if` anyway)
967edb7
to
cfcd43b
Compare
if (stepResult != null) { | ||
return stepResult | ||
} | ||
return null // TODO should this throw an exception? also look into `EventPayload` https://github.com/inngest/inngest-kt/pull/26#discussion_r150176713 |
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.
🚫 [ktlint] standard:max-line-length reported by reviewdog 🐶
Exceeded max line length (120)
https://github.com/inngest/inngest/blob/main/docs/SDK_SPEC.md#533-wait-for-event
https://www.inngest.com/docs/reference/functions/step-wait-for-event#step-wait-for-event-id-options-promise-null-event-payload
Currently only supporting the
if
optional condition, will addmatch
later (
match
is still sent over the wire as anif
anyway)