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

[GH-1604] Add error handling to Slack service #584

Merged
merged 9 commits into from
Mar 2, 2022

Conversation

okotsopoulos
Copy link
Contributor

@okotsopoulos okotsopoulos commented Feb 24, 2022

Purpose

The Slack token is stored in Vault. We now read it into environment variable WFL_SLACK_TOKEN.

Atlantis syncs the value from Vault into GitHub Secrets, and we use it to set the environment variable as needed within GitHub Actions: https://github.com/broadinstitute/terraform-ap-deployments/pull/554

Updated documentation.

Screen Shot 2022-02-25 at 6 15 07 PM

Catch throwables within slack service, and add unit test.

System Tests

Start a local server with Slack notifications enabled:

$ WFL_SLACK_ENABLED=enabled ./ops/server.sh 

Run system test known to emit Slack message off of local server:

$ cd /path/to/wfl/api;
$ WFL_WFL_URL=http://localhost:3000 clojure -M:test --focus wfl.system.v1-endpoint-test/test-workload-sink-outputs-to-tdr

Resulting Slack message, and test passes:

--- system (clojure.test) ---------------------------
wfl.system.v1-endpoint-test
  test-workload-sink-outputs-to-tdr

1 tests, 7 assertions, 0 failures.

Then ran the full suite, all pass:

$ make TARGET=system
export CPCACHE=/Users/okotsopo/wfl/api/.cpcache;            \
	export WFL_WFL_URL=http://localhost:3000; \
	clojure  -M:parallel-test wfl.system.v1-endpoint-test | \
	tee /Users/okotsopo/wfl/derived/api/system.log
WARNING: Specified path is external to project: ../derived/api/src
WARNING: Specified path is external to project: ../derived/api/resources

Ran 33 tests containing 374 assertions.
0 failures, 0 errors.
api system finished on Mon Feb 28 14:07:14 EST 2022
docs system finished on Mon Feb 28 14:07:14 EST 2022
functions/aou system finished on Mon Feb 28 14:07:14 EST 2022
functions/sg system finished on Mon Feb 28 14:07:14 EST 2022
helm system finished on Mon Feb 28 14:07:14 EST 2022
ui system finished on Mon Feb 28 14:07:14 EST 2022

Review Instructions

  • Take a look at the modified / added automated tests as a guide to slack usage, expected behavior.
  • Please read through the Atlantis documentation I added, and let me know if it feels sufficient if you were working in that space.

wfl.integration.slack-test/test-send-notification is passing locally
but failing in GitHub Actions:
403 - Vault API error - permission denied
@okotsopoulos okotsopoulos marked this pull request as ready for review February 28, 2022 18:19
Copy link
Contributor

@tbl3rd tbl3rd left a comment

Choose a reason for hiding this comment

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

Still poking around, but not sure when I'll get back to it.

api/src/wfl/service/slack.clj Outdated Show resolved Hide resolved
api/src/wfl/service/slack.clj Outdated Show resolved Hide resolved
api/src/wfl/service/slack.clj Outdated Show resolved Hide resolved
api/src/wfl/service/slack.clj Show resolved Hide resolved
api/test/wfl/integration/slack_test.clj Outdated Show resolved Hide resolved
api/test/wfl/integration/slack_test.clj Show resolved Hide resolved
api/src/wfl/service/slack.clj Outdated Show resolved Hide resolved
api/test/wfl/integration/slack_test.clj Show resolved Hide resolved
api/src/wfl/service/slack.clj Outdated Show resolved Hide resolved
api/test/wfl/integration/slack_test.clj Outdated Show resolved Hide resolved
@okotsopoulos okotsopoulos merged commit acb5cde into develop Mar 2, 2022
@okotsopoulos okotsopoulos deleted the okotsopo/GH-1604-try-slack-ops branch March 2, 2022 20:25
okotsopoulos added a commit that referenced this pull request Mar 4, 2022
GH-1629 TerraExecutor gets method configuration version from Firecloud (#585)
GH-1553 GH-1604 Add error handling to Slack service (#584)
GH-1628 Look at the first 7 workloads returned in test-workflows-by-filters. (#583)
GH-1617 Consistent workload representation for logs (#579)
GH-1624 Rename covid -> staged in code, docs (#580)
okotsopoulos added a commit that referenced this pull request Mar 7, 2022
GH-1629 TerraExecutor gets method configuration version from Firecloud (#585)
GH-1553 GH-1604 Add error handling to Slack service (#584)
GH-1628 Look at the first 7 workloads returned in test-workflows-by-filters. (#583)
GH-1617 Consistent workload representation for logs (#579)
GH-1624 Rename covid -> staged in code, docs (#580)
okotsopoulos added a commit that referenced this pull request Mar 7, 2022
GH-1629 TerraExecutor gets method configuration version from Firecloud (#585)
GH-1553 GH-1604 Add error handling to Slack service (#584)
GH-1628 Look at the first 7 workloads returned in test-workflows-by-filters. (#583)
GH-1617 Consistent workload representation for logs (#579)
GH-1624 Rename covid -> staged in code, docs (#580)
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