#174 script to retry failed webhook #18

GauravGusain98 · 2021-12-02T06:46:21Z

DostEducation/RP_IVR_analytics#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.

api/services/call_log_event_service.py

api/services/handle_event_service.py

GauravGusain98 · 2021-12-02T09:36:54Z

api/services/transaction_log_service.py

+    def get_failed_ivr_transaction_log(self):
+        failed_ivr_transaction_logs = (
+            models.IvrCallbackTransactionLog.query.filter_by(processed=False)
+            .limit(app.config["RETRY_LOGS_LIMIT"])


using env for limit

@GauravGusain98 though this looks good, the terminology seems a bit confusing.

Initially, I thought we are using this to limit the number of reattempts for retry.

Are we planning to have the retry limit?

Can we change the ENV variable name a bit? maybe like RETRY_LOGS_BATCH_LIMIT

Yes In case the logs are too many.

👍

GauravGusain98 · 2021-12-02T09:42:48Z

config.py

@@ -24,7 +24,7 @@
 }
 # For socket based connection
 SQLALCHEMY_DATABASE_URI = (
-    "postgresql://%(user)s:%(password)s/%(database)s?host=%(connection_name)s/"
+    "postgresql://%(user)s:%(password)s@/%(database)s?host=%(connection_name)s/"


Changes related to hotfix. Made another PR for the same.

GauravGusain98 · 2021-12-02T11:54:53Z

main.py

+    failed_ivr_logs = transaction_log_service.get_failed_ivr_transaction_log()
+
+    for log in failed_ivr_logs:
+        processed = process_form_data(json.loads(log.payload), log.created_on)


processing each failed log and passing the actual time that be in the created_on column of the new records that will be created.

Is there any reason to pass created_on separately? Isn't it possible to get it from JSON itself?

…lback into feature/174-script-to-retry-failed-webhook

Satendra-SR · 2021-12-03T05:35:58Z

api/services/handle_event_service.py


                """PickTime is available only when the call is answered.
                The telco code for answered calls is 16.
                """
-                if request.form["TelcoCode"] == "16":
-                    data["pick_time"] = request.form["PickTime"]
+                if form_data["TelcoCode"] == "16":


@GauravGusain98 Could you move this to the config file? It was there earlier, but let's clean whatever we can.

Satendra-SR · 2021-12-03T05:36:36Z

api/services/transaction_log_service.py

+    def get_failed_ivr_transaction_log(self):
+        failed_ivr_transaction_logs = (
+            models.IvrCallbackTransactionLog.query.filter_by(processed=False)
+            .limit(app.config["RETRY_LOGS_LIMIT"])


@GauravGusain98 though this looks good, the terminology seems a bit confusing.

Initially, I thought we are using this to limit the number of reattempts for retry.

Are we planning to have the retry limit?

Can we change the ENV variable name a bit? maybe like RETRY_LOGS_BATCH_LIMIT

api/services/transaction_log_service.py

Satendra-SR · 2021-12-03T06:25:15Z

main.py

+    failed_ivr_logs = transaction_log_service.get_failed_ivr_transaction_log()
+
+    for log in failed_ivr_logs:
+        processed = process_form_data(json.loads(log.payload), log.created_on)


Is there any reason to pass created_on separately? Isn't it possible to get it from JSON itself?

…lback into feature/174-script-to-retry-failed-webhook

Satendra-SR

One change is required to reduce the number of DB queries per log processing. Please fix and merge.

Satendra-SR · 2021-12-03T11:06:52Z

main.py

+    for log in failed_ivr_logs:
+        log.attempts += 1
+        db.session.add(log)
+        db.session.commit()
+
+        payload = json.loads(log.payload)
+        payload["log_created_on"] = log.created_on
+        processed = process_form_data(payload)
+
+        if processed is not True:
+            continue
+
+        log.processed = True
+        db.session.add(log)
+        db.session.commit()


We can reduce the number of database query per execution. Thoughts?

Suggested change

for log in failed_ivr_logs:

log.attempts += 1

db.session.add(log)

db.session.commit()

payload = json.loads(log.payload)

payload["log_created_on"] = log.created_on

processed = process_form_data(payload)

if processed is not True:

continue

log.processed = True

db.session.add(log)

db.session.commit()

for log in failed_ivr_logs:

payload = json.loads(log.payload)

payload["log_created_on"] = log.created_on

log.processed = process_form_data(payload)

log.attempts += 1

db.session.add(log)

db.session.commit()

In this way, if the logic is not updating the status?

Yea, the log.processed should be either True or False. And if you seeprocess_form_data(payload), it returns a boolean value. So the status will be based on the return type. Do you think this doesn't work?

GauravGusain98 added 4 commits December 1, 2021 17:25

added logic for retrying the failed webhooks

3e6fb96

added created_on fields while retrying the log

1826e1c

code cleanups

87ac70f

added retry log limit in env file

c12e0b0

GauravGusain98 requested review from rudresh and Satendra-SR December 2, 2021 06:46

GauravGusain98 commented Dec 2, 2021

View reviewed changes

api/services/call_log_event_service.py Show resolved Hide resolved

GauravGusain98 commented Dec 2, 2021

View reviewed changes

api/services/handle_event_service.py Show resolved Hide resolved

GauravGusain98 commented Dec 2, 2021

View reviewed changes

GauravGusain98 marked this pull request as ready for review December 2, 2021 11:55

Merge branch 'develop' of https://github.com/DostEducation/RP_IVR_cal…

020ddb2

…lback into feature/174-script-to-retry-failed-webhook

Satendra-SR reviewed Dec 3, 2021

View reviewed changes

GauravGusain98 added 4 commits December 3, 2021 12:22

code cleanups

f2461a2

Merge branch 'develop' of https://github.com/DostEducation/RP_IVR_cal…

65af1c7

…lback into feature/174-script-to-retry-failed-webhook

added check for reattempts of logs

71ae357

added fallback value in config variables

f83a2fc

Satendra-SR approved these changes Dec 3, 2021

View reviewed changes

GauravGusain98 added 2 commits December 3, 2021 17:12

code optimization

7217958

minor fix

4ca51c2

GauravGusain98 merged commit 89d0611 into develop Dec 3, 2021

GauravGusain98 deleted the feature/174-script-to-retry-failed-webhook branch December 3, 2021 11:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#174 script to retry failed webhook #18

#174 script to retry failed webhook #18

GauravGusain98 commented Dec 2, 2021 •

edited

GauravGusain98 Dec 2, 2021

Satendra-SR Dec 3, 2021

GauravGusain98 Dec 3, 2021

GauravGusain98 Dec 2, 2021

GauravGusain98 Dec 2, 2021

Satendra-SR Dec 3, 2021

GauravGusain98 Dec 3, 2021 •

edited

Satendra-SR Dec 3, 2021

Satendra-SR Dec 3, 2021

Satendra-SR Dec 3, 2021

Satendra-SR left a comment

Satendra-SR Dec 3, 2021

GauravGusain98 Dec 3, 2021

Satendra-SR Dec 3, 2021

#174 script to retry failed webhook #18

#174 script to retry failed webhook #18

Conversation

GauravGusain98 commented Dec 2, 2021 • edited

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

Types of changes

Short description of what this resolves:

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GauravGusain98 Dec 3, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Satendra-SR left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GauravGusain98 commented Dec 2, 2021 •

edited

GauravGusain98 Dec 3, 2021 •

edited