Skip to content

for call fail attempts try another 10 times and then stop#9

Merged
shmuelko merged 4 commits intomasterfrom
keep-running-with-500
Mar 1, 2021
Merged

for call fail attempts try another 10 times and then stop#9
shmuelko merged 4 commits intomasterfrom
keep-running-with-500

Conversation

@shmuelko
Copy link
Copy Markdown
Contributor

No description provided.

(if (nil? uri)
(loop [results []
uri uri
attempts 10
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please use constant here (similar to backoff-timeout)

;; load balancer freaking out, lets try again
(Thread/sleep 1000)
(recur results uri params))
(recur results uri (dec attempts) params))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You don't test if attempts is greater than 0 here

(log/warnf "API rate limit reached, waiting for %s seconds" backoff-timeout)
(Thread/sleep (* backoff-timeout 1000))
(recur results uri params))
(recur results (dec attempts) uri params))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

here too, you don't check if attempts is positive

(concat results data)
(conj results data)))
(get-in body [:links :next])
(dec attempts)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do you decrement attempts here?

;; utils

(def backoff-timeout "Backoff timeout in seconds" 20)
(def num-attemps "Number of attempts to run" 10)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

typo: num-attempts (missing t before s)
Actually better call it max-attempts

@shmuelko shmuelko merged commit 529118b into master Mar 1, 2021
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