daemon: send slack notifications (#281) #585

terryz21 · 2019-02-24T11:55:45Z

🎟️ Ticket(s): Closes #281, created #621 for follow-up

👷 Changes

A new file called Notifier was created in daemon to contain Notifier.go which handles how slack notifications are sent via Incoming Webhooks.
Notify() method called in Deploy() and makes POST request to send message to slack
For now, only notifies "Build started" & "Build completed"

🔦 Testing Instructions

Explain how to test your changes, if applicable.

Need create your own Slack workspace and install your own slack app. Follow the instructions in this link: https://api.slack.com/incoming-webhooks#advanced_message_formatting
Once you create your own Slack app, copy the generated Webhook URL for the channel you want to post and set it as the url field in NewNotifier method
Run tests in deployment_test.go

bobheadxi

this is a great start, awesome work @terryz21 😁 I've left a few comments - let me know if you have any questions!

daemon/inertiad/main.go

bobheadxi · 2019-02-24T20:12:25Z

daemon/inertiad/notifier/notifier.go

+		return err
+	}
+
+	_ = resp //note: temporary, may need response in the future?


we definitely want to check the resp here, at least for status codes (ie if its not 2xx then Slack rejected the request)

daemon/inertiad/notifier/notifier.go

bobheadxi · 2019-02-24T20:16:18Z

daemon/inertiad/notifier/notifier.go

+
+// NewNotifier creates a notifier with web hook url to slack channel
+func NewNotifier() *SlackNotifier {
+	url := "https://hooks.slack.com/services/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"


we probably want to make this URL configurable, ie with

func NewNotifier(webhook string) *SlackNotifier

also it's fine for now because the Slack workspace is a test one, but we want to avoid committing sensitive information into the repository (although this workspace might be useful for running integration tests, assuming it'll always be available for use)

another idea - if a empty string is provided, the notifier should become a no-op notifier (ie it doesn't do anything on Notify())

daemon/inertiad/project/deployment.go

daemon/inertiad/project/deployment_test.go

yaoharry

Should unstage unchanged files ex: *.png, compiled.go, id_rsa.

terryz21 · 2019-02-26T03:35:24Z

Thanks for the comments! I will make these changes along with some of the formatting additions this week

Co-Authored-By: terryz21 <[email protected]>

… daemon/#281-slack-notifier

…r configuring slack webhook URL.

… replaced with comma previously

codecov · 2019-03-30T11:12:44Z

Codecov Report

Merging #585 into master will decrease coverage by 0.08%.
The diff coverage is 25%.

@@            Coverage Diff             @@
##           master     #585      +/-   ##
==========================================
- Coverage   55.88%   55.81%   -0.07%     
==========================================
  Files          68       68              
  Lines        3377     3385       +8     
==========================================
+ Hits         1887     1889       +2     
- Misses       1248     1253       +5     
- Partials      242      243       +1

Impacted Files	Coverage Δ
daemon/inertiad/project/deployment.go	`33.79% <25%> (-0.33%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f592bb...7684d68. Read the comment docs.

bobheadxi · 2019-03-30T16:33:23Z

daemon/inertiad/notifier/notifier.go

+
+// Notify sends the notification
+func (n *SlackNotifier) Notify(text string, options *NotifyOptions) error {
+	// check if url is empty


only really need to leave comments if the code is not self-explanatory 😋

Suggested change

// check if url is empty

// check if url is empty

bobheadxi · 2019-03-30T16:33:47Z

daemon/inertiad/notifier/notifier.go

+		return err
+	}
+	defer resp.Body.Close()
+	body, err := ioutil.ReadAll(resp.Body)


should check error here

bobheadxi · 2019-03-30T16:34:09Z

daemon/inertiad/notifier/notifier.go

+	bodyString := string(body)
+
+	if resp.StatusCode < 200 || resp.StatusCode > 299 {
+		return errors.New("Http request rejected by Slack. Error: " + bodyString)


lowercase for go error convention:

Suggested change

return errors.New("Http request rejected by Slack. Error: " + bodyString)

return errors.New("http request rejected by Slack. Error: " + bodyString)

bobheadxi

almost there 😁

…281-slack-notifier

bobheadxi · 2019-10-02T07:28:14Z

We've been flagged for abuse again, so travis builds aren't running... ugh

daemon: send slack notifications (#281)

39dc311

terryz21 requested a review from bobheadxi as a code owner February 24, 2019 11:55

terryz21 requested a review from a team February 24, 2019 11:55

ghost requested review from kimoantiqe and seifghazi February 24, 2019 11:55

bobheadxi requested changes Feb 24, 2019

View reviewed changes

bobheadxi requested a review from yaoharry February 24, 2019 20:20

bobheadxi added the pr: wip in progress but seeking feedback label Feb 24, 2019

yaoharry requested changes Feb 24, 2019

View reviewed changes

bobheadxi and others added 7 commits February 25, 2019 19:41

Update daemon/inertiad/project/deployment_test.go

7035d5f

Co-Authored-By: terryz21 <[email protected]>

Update daemon/inertiad/notifier/notifier.go

1cf97e3

Co-Authored-By: terryz21 <[email protected]>

Update daemon/inertiad/project/deployment.go

2fe3e3a

Co-Authored-By: terryz21 <[email protected]>

Merge branch 'master' of https://github.com/ubclaunchpad/inertia into…

6fcd03e

… daemon/#281-slack-notifier

Added some formatting for the slack messages sent. Missing changes fo…

226802f

…r configuring slack webhook URL.

Resolving syntax error on line 41. Accidently deleted one bracket and…

68e822e

… replaced with comma previously

removing unchanged files .png* and id_rsa from pull request

662eee8

Forgot to delete hard coded value for test in Attachments json object

d0c80de

bobheadxi reviewed Mar 30, 2019

View reviewed changes

Terry and others added 4 commits May 10, 2019 23:48

Added error check for HTTPresponse body

b366ffc

Merge branch 'master' of github.com:ubclaunchpad/inertia into daemon/#…

158c283

…281-slack-notifier

chore: sync up .static with master

4820b2d

big cleanup

da5973c

bobheadxi self-assigned this Oct 2, 2019

bobheadxi removed the pr: wip in progress but seeking feedback label Oct 2, 2019

bobheadxi added the pr: finalized needs review and final approval label Oct 2, 2019

bobheadxi mentioned this pull request Oct 2, 2019

daemon: integrate slack notification with config cycle #621

Closed

bobheadxi approved these changes Oct 2, 2019

View reviewed changes

bobheadxi added 2 commits October 3, 2019 01:14

trigger build

6d857f7

trigger build

7684d68

bobheadxi merged commit 13389b4 into master Oct 3, 2019

bobheadxi deleted the daemon/#281-slack-notifier branch October 3, 2019 21:50

bobheadxi mentioned this pull request Oct 21, 2019

daemon: build and deploy notifications #194

Open

4 tasks

bobheadxi mentioned this pull request Mar 22, 2020

daemon(deployment): integrate slack config #661

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

daemon: send slack notifications (#281) #585

daemon: send slack notifications (#281) #585

terryz21 commented Feb 24, 2019 •

edited by bobheadxi

Loading

bobheadxi left a comment

bobheadxi Feb 24, 2019

bobheadxi Feb 24, 2019

bobheadxi Feb 24, 2019 •

edited

Loading

yaoharry left a comment

terryz21 commented Feb 26, 2019 •

edited

Loading

codecov bot commented Mar 30, 2019 •

edited

Loading

bobheadxi Mar 30, 2019

bobheadxi Mar 30, 2019

bobheadxi Mar 30, 2019

bobheadxi left a comment

bobheadxi commented Oct 2, 2019

	return errors.New("Http request rejected by Slack. Error: " + bodyString)
	return errors.New("http request rejected by Slack. Error: " + bodyString)

daemon: send slack notifications (#281) #585

daemon: send slack notifications (#281) #585

Conversation

terryz21 commented Feb 24, 2019 • edited by bobheadxi Loading

👷 Changes

🔦 Testing Instructions

bobheadxi left a comment

Choose a reason for hiding this comment

bobheadxi Feb 24, 2019

Choose a reason for hiding this comment

bobheadxi Feb 24, 2019

Choose a reason for hiding this comment

bobheadxi Feb 24, 2019 • edited Loading

Choose a reason for hiding this comment

yaoharry left a comment

Choose a reason for hiding this comment

terryz21 commented Feb 26, 2019 • edited Loading

codecov bot commented Mar 30, 2019 • edited Loading

Codecov Report

bobheadxi Mar 30, 2019

Choose a reason for hiding this comment

bobheadxi Mar 30, 2019

Choose a reason for hiding this comment

bobheadxi Mar 30, 2019

Choose a reason for hiding this comment

bobheadxi left a comment

Choose a reason for hiding this comment

bobheadxi commented Oct 2, 2019

terryz21 commented Feb 24, 2019 •

edited by bobheadxi

Loading

bobheadxi Feb 24, 2019 •

edited

Loading

terryz21 commented Feb 26, 2019 •

edited

Loading

codecov bot commented Mar 30, 2019 •

edited

Loading