Skip to content
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

#174 script to retry failed webhooks #215

Merged
merged 9 commits into from
Dec 3, 2021

Conversation

GauravGusain98
Copy link
Contributor

@GauravGusain98 GauravGusain98 commented Dec 2, 2021

#174

Please complete the following steps and check these boxes before filing your PR:

Types of changes

  • Bug fix (a change which fixes an issue)
  • New feature (a change which adds functionality)

Short description of what this resolves:

  • Added functionality to retry failed webhook logs
  • The created on a date is set to the created on the date of logs.

Checklist:

  • I have performed a self-review of my own code.
  • The code follows the style guidelines of this project.
  • The code changes are passing the CI checks
  • I have documented my code wherever required.
  • The changes requires a change to the documentation.
  • I have updated the documentation based on the my changes.

@GauravGusain98 GauravGusain98 changed the title Feature/174 script to retry failed webhook #174 script to retry failed webhooks Dec 2, 2021
Comment on lines +99 to +101
created_on=jsonData["log_created_on"]
if jsonData.get("log_created_on", None)
else datetime.now(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will only receive jsonData["log_created_on"] for the retry logic.

def get_failed_webhook_transaction_log(self):
failed_webhook_transaction_logs = (
models.WebhookTransactionLog.query.filter_by(processed=False)
.limit(app.config["RETRY_LOGS_LIMIT"])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

taking limit of the failed logs count using env

):
handle_user_custom_field_data(jsonData)
webhook_log = transaction_log_service.create_new_webhook_log(jsonData)
processed = handle_payload(jsonData)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inside handle_payload function try and except are used for error handling and based on that it returns True or False

if "contact" in jsonData:
transaction_log_service.mark_webhook_log_as_processed(webhook_log)
elif jsonData and jsonData.get("type", None) == "retry_failed_log":
retry_failed_webhook(transaction_log_service)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call retry login if we have "type": "retry_failed_log" in json data.

for log in failed_webhook_logs:
json_data = json.loads(log.payload)
json_data["log_created_on"] = log.created_on
processed = handle_payload(json_data, True)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Processing all the failed logs. The second parameter is for differentiating regular webhooks and retry webhooks

db_helper.save(log)


def handle_payload(jsonData, is_retry_payload=False):
Copy link
Contributor Author

@GauravGusain98 GauravGusain98 Dec 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initial logic for handling webhooks

@GauravGusain98 GauravGusain98 marked this pull request as ready for review December 2, 2021 09:01
Copy link
Member

@Satendra-SR Satendra-SR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one suggestion else looks good. Please fix and merge

config.py Outdated
Comment on lines 41 to 42
RETRY_LOGS_BATCH_LIMIT = os.environ.get("RETRY_LOGS_BATCH_LIMIT")
MAX_RETRY_ATTEMPTS_FOR_LOGS = os.environ.get("MAX_RETRY_ATTEMPTS_FOR_LOGS")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a fallback value (kind of default value) like you must have seen frameworks config files?

@GauravGusain98 GauravGusain98 merged commit ff0c187 into develop Dec 3, 2021
@GauravGusain98 GauravGusain98 deleted the feature/174-script-to-retry-failed-webhook branch December 3, 2021 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants