Post a message to slack #90

AmanRaj1608 · 2019-07-03T17:10:05Z

Send message to slack when someone gets invited

Fixes #89

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Non Functional Requirement

Follows the code style of this project.
Tests Cover Changes
I have performed a self-review of my own code
All new and existing tests passed.
Documentation

Send message to slack when someone gets invited #89

yashLadha

PTAL

yashLadha · 2019-07-03T17:21:38Z

server.js

+    })
+    .catch(error => {
+      console.log('FAILED: Send slack webhook', error)
+      // reject(new Error('FAILED: Send slack webhook'))


In case of failure, we can work on max_retry logic. Sometimes what happens is that when slack gets more load, it puts the requests in queue and sometimes those requests are also dropped. So to ensure correctness from our side we can rely on max-retry logic and can store the value in some constant.

Thanks, I will read about this and fix this issue.

In case of failure, we can redirect error to another channel too, if we want to.

Yes, even after themax_retry we can send a single request to post error on a different slack channel. No need to use max_retry logic there because it won't benefit to waste resources on successfully sending of error 😉

@yashLadha
I don't know how to do it using axios 😅
If you could send some reference , It will be very helpful

Try to look for the retry params, a quick google search will get you to the solution. If you are still not able to find out, ping me I will tell you.

server.js

aashutoshrathi

I tested the code locally, worked fine.
here's output: https://iiitvadodara.slack.com/archives/CKP5MG4LA/p1562185719000100

Left a few comments.

server.js

aashutoshrathi · 2019-07-03T20:33:49Z

server.js

+    })
+    .catch(error => {
+      console.log('FAILED: Send slack webhook', error)
+      // reject(new Error('FAILED: Send slack webhook'))


In case of failure, we can redirect error to another channel too, if we want to.

Added 1. dotenv 2. Change message 3. Fix link

AmanRaj1608 · 2019-07-05T12:45:32Z

@yashLadha
Sir I tried this logic which I found here axios/axios#164 (comment)

  axios.post(webhookURL, JSON.stringify(options),
    { retry: 5, retryDelay: 1000 })
    .then(response => {
      console.log('SUCCESS: Sent slack webhook:', response.data)
    })
    .catch(error => {
      console.log('FAILED: Sent slack webhook', error)
    })

I also find this amazing package https://www.npmjs.com/package/axios-retry .
This npm package is quite good

Which one should I use

npm
axios-retry
Axios plugin that intercepts failed requests and retries them whenever posible.

aashutoshrathi

LGTM, just add retry logic

yashLadha · 2019-07-05T20:27:18Z

Avoid, using new packages. Try to implement the functionality using the same package.

…

On Fri, Jul 5, 2019 at 8:58 PM Aashutosh Rathi ***@***.***> wrote: ***@***.**** approved this pull request. LGTM, just add retry logic — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#90?email_source=notifications&email_token=AEJSUT5IYPP43TT2SFYH7WLP55SB5A5CNFSM4H5H444KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB5T4V7A#pullrequestreview-258460412>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEJSUT6JZFLM5UBPWIIAD2TP55SB5ANCNFSM4H5H444A> .

-- Yash Ladha

Added max_retry logic to send request 3 times in case first failed.

AmanRaj1608 · 2019-07-06T12:57:30Z

@yashLadha
Please review 😀

yashLadha

Promises aren't synchronous. PTAL

server.js

Fixed asynchronous behaviour

AmanRaj1608 · 2019-07-08T08:03:46Z

@yashLadha
Sir it's working properly now please review again 😅. It will send message only one time if URL is correct and fails for 3 times if I enter incorrect URL.

server.js

yashLadha

PTAL, else all looks good to me.

AmanRaj1608 · 2019-07-08T18:55:05Z

@yashLadha
Done 😀

aashutoshrathi

Why aren't we using async-await?

aashutoshrathi · 2019-07-08T19:09:35Z

Also, can you once check the file that all variable names are camelCase.
Thanks

AmanRaj1608 · 2019-07-08T19:20:32Z

Why aren't we using async-await?

I tried using async-await but somehow it was not working properly. It was like

flag 0
flag 1
flag 2
SUCCESS: Sent slack webhook
SUCCESS: Sent slack webhook
SUCCESS: Sent slack webhook

I am not sure what I was doing wrong. So I ended up doing like this 😥.

Also, can you once check the file that all variable names are camelCase.
Thanks

Yes I followed code style as ES Lint yells if I don't follow it. And before pushing code I did used npm run lint:fix.

server.js

yashLadha

LGTM

post message to slack

4c87f57

Send message to slack when someone gets invited #89

yashLadha requested changes Jul 3, 2019

View reviewed changes

aashutoshrathi requested changes Jul 3, 2019

View reviewed changes

AmanRaj1608 and others added 2 commits July 5, 2019 17:36

minor fix

c1b6d47

Added 1. dotenv 2. Change message 3. Fix link

Merge branch 'master' into master

0eebe28

aashutoshrathi approved these changes Jul 5, 2019

View reviewed changes

max_retry logic

af73bde

Added max_retry logic to send request 3 times in case first failed.

yashLadha requested changes Jul 7, 2019

View reviewed changes

server.js Outdated Show resolved Hide resolved

max_retry working ✔

f82bc9d

Fixed asynchronous behaviour

yashLadha reviewed Jul 8, 2019

View reviewed changes

server.js Outdated Show resolved Hide resolved

yashLadha requested changes Jul 8, 2019

View reviewed changes

Retry logic

6680e7e

aashutoshrathi reviewed Jul 8, 2019

View reviewed changes

server.js Outdated Show resolved Hide resolved

fix camelCase

806ca18

yashLadha approved these changes Jul 9, 2019

View reviewed changes

aashutoshrathi merged commit 6dd403f into iiitv:master Jul 9, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Post a message to slack #90

Post a message to slack #90

AmanRaj1608 commented Jul 3, 2019 •

edited

yashLadha left a comment

yashLadha Jul 3, 2019

AmanRaj1608 Jul 3, 2019

aashutoshrathi Jul 3, 2019

yashLadha Jul 4, 2019

AmanRaj1608 Jul 4, 2019

yashLadha Jul 4, 2019

aashutoshrathi left a comment

aashutoshrathi Jul 3, 2019

AmanRaj1608 commented Jul 5, 2019 •

edited by unfurl-links bot

aashutoshrathi left a comment

yashLadha commented Jul 5, 2019 via email

AmanRaj1608 commented Jul 6, 2019

yashLadha left a comment

AmanRaj1608 commented Jul 8, 2019

yashLadha left a comment

AmanRaj1608 commented Jul 8, 2019

aashutoshrathi left a comment

aashutoshrathi commented Jul 8, 2019

AmanRaj1608 commented Jul 8, 2019 •

edited

yashLadha left a comment

Post a message to slack #90

Post a message to slack #90

Conversation

AmanRaj1608 commented Jul 3, 2019 • edited

Send message to slack when someone gets invited

Fixes #89

Types of changes

Non Functional Requirement

yashLadha left a comment

Choose a reason for hiding this comment

yashLadha Jul 3, 2019

Choose a reason for hiding this comment

AmanRaj1608 Jul 3, 2019

Choose a reason for hiding this comment

aashutoshrathi Jul 3, 2019

Choose a reason for hiding this comment

yashLadha Jul 4, 2019

Choose a reason for hiding this comment

AmanRaj1608 Jul 4, 2019

Choose a reason for hiding this comment

yashLadha Jul 4, 2019

Choose a reason for hiding this comment

aashutoshrathi left a comment

Choose a reason for hiding this comment

aashutoshrathi Jul 3, 2019

Choose a reason for hiding this comment

AmanRaj1608 commented Jul 5, 2019 • edited by unfurl-links bot

aashutoshrathi left a comment

Choose a reason for hiding this comment

yashLadha commented Jul 5, 2019 via email

AmanRaj1608 commented Jul 6, 2019

yashLadha left a comment

Choose a reason for hiding this comment

AmanRaj1608 commented Jul 8, 2019

yashLadha left a comment

Choose a reason for hiding this comment

AmanRaj1608 commented Jul 8, 2019

aashutoshrathi left a comment

Choose a reason for hiding this comment

aashutoshrathi commented Jul 8, 2019

AmanRaj1608 commented Jul 8, 2019 • edited

yashLadha left a comment

Choose a reason for hiding this comment

AmanRaj1608 commented Jul 3, 2019 •

edited

AmanRaj1608 commented Jul 5, 2019 •

edited by unfurl-links bot

AmanRaj1608 commented Jul 8, 2019 •

edited