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

fix: pass slack recipients correctly #29721

Merged
merged 2 commits into from
Aug 2, 2024
Merged

fix: pass slack recipients correctly #29721

merged 2 commits into from
Aug 2, 2024

Conversation

eschutho
Copy link
Member

@eschutho eschutho commented Jul 26, 2024

SUMMARY

There was a bug where the slack recipients weren't getting updated correctly when they changed from v1 name to v2 ids during execution. This also adds slack channels to header logs so that if there is an error of channel_not_found for example, we can check the logs to see which channels were passed in. And lastly I found that the integration tests weren't loading user data, so I fixed that as well.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

Run a test with slack and check that logs include this new information.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@dosubot dosubot bot added the logging Creates a UI or API endpoint that could benefit from logging. label Jul 26, 2024
@eschutho eschutho changed the title chore: add slack channels to header data logs fix: pass slack recipients correctly Jul 26, 2024
@sadpandajoe
Copy link
Member

@supersetbot label 4.1

@github-actions github-actions bot added the v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch label Jul 30, 2024
@eschutho eschutho force-pushed the elizabeth/slack-logs branch 2 times, most recently from b6ceefc to 338ed7a Compare August 1, 2024 20:15
@eschutho eschutho force-pushed the elizabeth/slack-logs branch 5 times, most recently from a2885ee to 991aa53 Compare August 1, 2024 21:19
@@ -230,6 +230,7 @@ module = "tests.*"
check_untyped_defs = false
disallow_untyped_calls = false
disallow_untyped_defs = false
disable_error_code = "annotation-unchecked"
Copy link
Member Author

Choose a reason for hiding this comment

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

This was needed to skip the error: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked] It seems like it should be covered in check_untyped_defs = false but it is not.

@@ -53,6 +53,9 @@ function test_init() {
echo Superset init
echo --------------------
superset init
echo Load test users
echo --------------------
superset load-test-users
Copy link
Member Author

Choose a reason for hiding this comment

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

small fix after an earlier refactor where the test script stopped loading test users.

@sadpandajoe
Copy link
Member

Nice! I see some new tests in there too.

@eschutho eschutho merged commit 57e8cd2 into master Aug 2, 2024
33 checks passed
WanjohiWanjohi pushed a commit to IDinsight/surveystream_superset_source that referenced this pull request Aug 6, 2024
sadpandajoe pushed a commit that referenced this pull request Aug 13, 2024
@rusackas rusackas deleted the elizabeth/slack-logs branch September 27, 2024 20:54
@github-actions github-actions bot added 🍒 4.1.0 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels labels Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels logging Creates a UI or API endpoint that could benefit from logging. preset-io size/L v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch 🍒 4.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants