-
Notifications
You must be signed in to change notification settings - Fork 35
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
feat/submitter allow retry flag #1134
Conversation
queue-model/schema.prisma
Outdated
@@ -17,4 +17,5 @@ model Submission { | |||
return_reference String? | |||
complete Boolean @default(false) | |||
retry_counter Int | |||
allow_retry Boolean |
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 the @default(true)
annotation and regenerate the migration file (or manually change it). Otherwise when prisma attempt to run the migration on a non-empty table it'll start throwing errors
also congrats @default
on the gh handle
data: object, | ||
url?: string, | ||
allowRetry = true | ||
): Promise<QueueResponse> { | ||
const rowData = { | ||
data: JSON.stringify(data), | ||
created_at: new Date(), | ||
updated_at: new Date(), | ||
webhook_url: url ?? null, |
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.
sorry, I only just spotted this, is this actually nullable / allowed to be null?
we should set url
to non optional and webhook_url to non nullable
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.
ah yeah, I think initially I was thinking every form would put a line in the db and only wanted the ones that have a webhook url to be processed, but you're right we only push in submissions that have webhook urls
/* | ||
Warnings: | ||
|
||
- Made the column `webhook_url` on table `Submission` required. This step will fail if there are existing NULL values in that column. |
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.
mind addressing this warning as well please. default empty string is fine
…rl fields to an empty string before running table modification
Description
In some instances, it's necessary that a webhook only be tried once in the case that it fails. This feature upgrades the form runner and submitter to accept an
allowRetry
flag, set totrue
orfalse
, which determines whether the webhook should be tried more than once.model/schema/schema.ts
to allowallowRetry
on webhook outputConfiguration.allow_retry
allowRetry
if it's set on the output, or defaults totrue
if notType of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce
the testing if necessary.
Checklist: