feat: preserve alerts, update to 40sdk #97

michaelcramer · 2022-10-06T15:06:38Z

The changes here preserve alerts when importing dashboards.
Dashboard elements don't have slugs and the element id is updated when a dashboard is overwritten, which breaks original alerts.
The dashboard element id for an alert can't be updated, even though it is not listed in the docs as a readonly attribute.

In order to preserve alerts, we first get all of the existing alerts. Then as we import each dashboard, we save all of the element ids that have existing alerts. After import, we map those old element ids to the new ids, based on title and query_id. Finally we create new alerts using the new element ids.

Alerts will still break if there are title or query changes between the old and the updated dashboard, as there isn't a convenient way to match the element ids.

In order to use the alerts API, I also had to update deployer to use the 40 sdk.

github-actions · 2022-10-06T15:10:22Z

Python Tests

    2 files -     6 56 suites - 168 0s ⏱️ -4s
  23 tests -   64   1 ✔️ -   86 0 💤 ±0 11 ❌ +11 11 🔥 +11
104 runs - 460 51 ✔️ - 513 0 💤 ±0 42 ❌ +42 11 🔥 +11

For more details on these failures and errors, see this check.

Results for commit 809234e. ± Comparison against base commit 3065a4f.

This pull request removes 86 and adds 22 tests. Note that renamed tests count towards both.

tests.test_deploy_boards ‑ test_audit_board_no_misses
tests.test_deploy_boards ‑ test_audit_board_with_misses
tests.test_deploy_boards ‑ test_create_board_create_homepage_call
tests.test_deploy_boards ‑ test_create_board_item_dashboard
tests.test_deploy_boards ‑ test_create_board_item_dashboard_item_call
tests.test_deploy_boards ‑ test_create_board_item_dashboard_match_call
tests.test_deploy_boards ‑ test_create_board_item_look
tests.test_deploy_boards ‑ test_create_board_item_look_item_call
tests.test_deploy_boards ‑ test_create_board_item_look_match_call
tests.test_deploy_boards ‑ test_create_board_section
…

C901, &#x27;import_content&#x27; is too complex (21)
E231, missing whitespace after &#x27;,&#x27;
E261, at least two spaces before inline comment
E262, inline comment should start with &#x27;# &#x27;
E265, block comment should start with &#x27;# &#x27;
E275, missing whitespace after keyword
E302, expected 2 blank lines, found 1
E303, too many blank lines (2)
W291, trailing whitespace
W293, blank line contains whitespace
…

github-actions · 2022-10-07T21:01:24Z

Python Tests

    6 files ±  0 168 suites ±0 5s ⏱️ +3s
  58 tests - 29   25 ✔️ -   62 0 💤 ±0   32 ❌ +  32 1 🔥 +1
420 runs -   3 231 ✔️ - 192 0 💤 ±0 186 ❌ +186 3 🔥 +3

For more details on these failures and errors, see this check.

Results for commit 82d18a9. ± Comparison against base commit 1d080c2.

This pull request removes 40 and adds 11 tests. Note that renamed tests count towards both.

tests.test_deploy_content ‑ test_build_spaces
tests.test_deploy_content ‑ test_deploy_content_build_call
tests.test_deploy_content ‑ test_deploy_content_import_content_call
tests.test_deploy_content ‑ test_deploy_space_build_call
tests.test_deploy_content ‑ test_deploy_space_dashboard_call
tests.test_deploy_content ‑ test_deploy_space_look_call
tests.test_deploy_content ‑ test_import_content_win
tests.test_deploy_content_export ‑ test_export_space
tests.test_deploy_content_export ‑ test_export_space_debug
tests.test_deploy_content_export ‑ test_export_space_no_verify_ssl
…

C901, &#x27;import_content&#x27; is too complex (21)
E231, missing whitespace after &#x27;,&#x27;
E261, at least two spaces before inline comment
E262, inline comment should start with &#x27;# &#x27;
E265, block comment should start with &#x27;# &#x27;
E302, expected 2 blank lines, found 1
E303, too many blank lines (2)
W291, trailing whitespace
W293, blank line contains whitespace
W391, blank line at end of file
…

AdamMinton

Let me know what changes you want to handle and which I can handle. Thanks!

AdamMinton · 2022-11-04T02:11:46Z

looker_deployer/commands/deploy_content.py

@@ -303,6 +359,8 @@ def main(args):
        args.target_base = 'Shared'

    sdk = get_client(args.ini, args.env)
+    global enabled_alerts
+    enabled_alerts = sdk.search_alerts(disabled="false")


I believe you need sdk.search_alerts(disabled=False, all_owners=True) to ensure all alerts for all users are pulled in (same with schedules)

AdamMinton · 2022-11-04T02:14:59Z

looker_deployer/commands/deploy_content.py

+    existing_dash_alerts = []
+    existing_elements_w_alerts = []
+
+    if content_type == "dashboard":  ## only run for dashboards, looks can't have alerts


Is it possible to move this into a separate function that is called from here to allow for unit tests?

AdamMinton · 2022-11-04T02:16:10Z

looker_deployer/commands/deploy_content.py

+        if is_new_dash == "false": ## if it's an existing dashboard, save the alerts and elements
+            for element in existing_dash[0]['dashboard_elements']:
+                start = len(existing_dash_alerts)
+                alerts = list(filter(lambda alert: alert['dashboard_element_id'] == element['id'], enabled_alerts))


To get this to work I needed to do the following str(alert['dashboard_element_id']) == str(element['id']) I think the alert ID might be a number 🤦 this allowed for the alert to be found

AdamMinton · 2022-11-04T02:19:19Z

looker_deployer/commands/deploy_content.py

+            updated_dash = sdk.search_dashboards(title=json_dash['title'],folder_id=space_id)
+        old_to_new_ids = {}
+        for element in existing_elements_w_alerts:  ## match old element ids to new element ids
+            updated_dash_element = list(filter(lambda item: item['title'] == element['title'] and item['query_id'] == element['query_id'], updated_dash[0]['dashboard_elements']))


Okay so we're comparing title of the element and the query ID. What happens when the query ID changes? It's okay this break the alert as well? I also could not get this work when I tried to unit test it, didn't delve too deep into why. On 22.16 it's unsetting my alert from the dashboard but I'm also not getting it disabled either.

AdamMinton · 2022-11-04T02:20:55Z

looker_deployer/commands/deploy_content.py

+            for key in alert.keys():
+                new_alert[key] = alert[key]                
+            if alert['dashboard_element_id'] in old_to_new_ids.keys():
+                logger.debug("creating alert", extra={"old_element_id": alert['dashboard_element_id'], "new_element_id": old_to_new_ids[alert['dashboard_element_id']]})


Can we get an logger.info line that summarizes what is happening that specifies the dashboard element ID and alert being updated.

AdamMinton · 2022-11-04T02:22:41Z

looker_deployer/commands/deploy_content.py

+    disabled_alerts = sdk.search_alerts(disabled="true")
+    for alert in disabled_alerts:
+        if alert['disabled_reason'] == "Dashboard element has been removed.":
+            sdk.delete_alert(alert['id'])


Can we actually just disable the alert via update_alert, you can take the body and then change the is_disabled to false and add a disabled reason

AdamMinton · 2022-11-04T02:23:25Z

looker_deployer/commands/deploy_content.py

+    disabled_alerts = sdk.search_alerts(disabled="true")
+    for alert in disabled_alerts:
+        if alert['disabled_reason'] == "Dashboard element has been removed.":
+            sdk.delete_alert(alert['id'])


Also logger.info for any time we do delete or updating so people can see which IDs changed

AdamMinton · 2022-11-04T02:24:33Z

looker_deployer/commands/deploy_content.py

@@ -303,6 +359,8 @@ def main(args):
        args.target_base = 'Shared'

    sdk = get_client(args.ini, args.env)
+    global enabled_alerts


Can we can get an update to the readme about this new functionality

AdamMinton · 2022-11-04T02:24:52Z

looker_deployer/utils/get_client.py

-        auth_session.AuthSession(settings, transport, serialize.deserialize31, "3.1"),
-        serialize.deserialize31,
-        serialize.serialize31,
+    return methods.Looker40SDK(


Can we add this to the readme as well that we are moving as a whole to API 4.0

AdamMinton · 2022-11-04T02:25:51Z

looker_deployer/commands/deploy_content.py

+    for alert in disabled_alerts:
+        if alert['disabled_reason'] == "Dashboard element has been removed.":
+            sdk.delete_alert(alert['id'])
+

 def get_space_ids_from_name(space_name, parent_id, sdk):


Can we update any functions from space to folder and variables? Also tests will need to be updated for this change. Just don't change the actual looker commands (which already referred to things as folders). I'm pretty sure space to folder find & replace should fix 95% of use cases.

github-actions · 2023-01-10T15:20:12Z

Python Tests

    6 files ±  0 168 suites ±0 5s ⏱️ +3s
  58 tests - 29   25 ✔️ -   62 0 💤 ±0   32 ❌ +  32 1 🔥 +1
444 runs +21 231 ✔️ - 192 0 💤 ±0 210 ❌ +210 3 🔥 +3

For more details on these failures and errors, see this check.

Results for commit 4d772c4. ± Comparison against base commit 1d080c2.

This pull request removes 40 and adds 11 tests. Note that renamed tests count towards both.

tests.test_deploy_content ‑ test_build_spaces
tests.test_deploy_content ‑ test_deploy_content_build_call
tests.test_deploy_content ‑ test_deploy_content_import_content_call
tests.test_deploy_content ‑ test_deploy_space_build_call
tests.test_deploy_content ‑ test_deploy_space_dashboard_call
tests.test_deploy_content ‑ test_deploy_space_look_call
tests.test_deploy_content ‑ test_import_content_win
tests.test_deploy_content_export ‑ test_export_space
tests.test_deploy_content_export ‑ test_export_space_debug
tests.test_deploy_content_export ‑ test_export_space_no_verify_ssl
…

C901, &#x27;import_content&#x27; is too complex (21)
E231, missing whitespace after &#x27;,&#x27;
E261, at least two spaces before inline comment
E262, inline comment should start with &#x27;# &#x27;
E265, block comment should start with &#x27;# &#x27;
E302, expected 2 blank lines, found 1
E303, too many blank lines (2)
W291, trailing whitespace
W293, blank line contains whitespace
W391, blank line at end of file
…

github-actions · 2023-01-11T19:12:37Z

Python Tests

    6 files ±  0 168 suites ±0 5s ⏱️ +3s
  58 tests - 29   25 ✔️ -   62 0 💤 ±0   32 ❌ +  32 1 🔥 +1
444 runs +21 231 ✔️ - 192 0 💤 ±0 210 ❌ +210 3 🔥 +3

For more details on these failures and errors, see this check.

Results for commit b2d7955. ± Comparison against base commit 1d080c2.

This pull request removes 40 and adds 11 tests. Note that renamed tests count towards both.

tests.test_deploy_content ‑ test_build_spaces
tests.test_deploy_content ‑ test_deploy_content_build_call
tests.test_deploy_content ‑ test_deploy_content_import_content_call
tests.test_deploy_content ‑ test_deploy_space_build_call
tests.test_deploy_content ‑ test_deploy_space_dashboard_call
tests.test_deploy_content ‑ test_deploy_space_look_call
tests.test_deploy_content ‑ test_import_content_win
tests.test_deploy_content_export ‑ test_export_space
tests.test_deploy_content_export ‑ test_export_space_debug
tests.test_deploy_content_export ‑ test_export_space_no_verify_ssl
…

C901, &#x27;import_content&#x27; is too complex (21)
E231, missing whitespace after &#x27;,&#x27;
E261, at least two spaces before inline comment
E262, inline comment should start with &#x27;# &#x27;
E265, block comment should start with &#x27;# &#x27;
E302, expected 2 blank lines, found 1
E303, too many blank lines (2)
W291, trailing whitespace
W293, blank line contains whitespace
W391, blank line at end of file
…

github-actions · 2023-02-15T17:34:54Z

Python Tests

    4 files -     2 112 suites - 56 3s ⏱️ -1s
  57 tests -   30   25 ✔️ -   62 0 💤 ±0   31 ❌ +  31 1 🔥 +1
292 runs - 131 154 ✔️ - 269 0 💤 ±0 136 ❌ +136 2 🔥 +2

For more details on these failures and errors, see this check.

Results for commit 17c4d4a. ± Comparison against base commit 1d080c2.

This pull request removes 40 and adds 10 tests. Note that renamed tests count towards both.

tests.test_deploy_content ‑ test_build_spaces
tests.test_deploy_content ‑ test_deploy_content_build_call
tests.test_deploy_content ‑ test_deploy_content_import_content_call
tests.test_deploy_content ‑ test_deploy_space_build_call
tests.test_deploy_content ‑ test_deploy_space_dashboard_call
tests.test_deploy_content ‑ test_deploy_space_look_call
tests.test_deploy_content ‑ test_import_content_win
tests.test_deploy_content_export ‑ test_export_space
tests.test_deploy_content_export ‑ test_export_space_debug
tests.test_deploy_content_export ‑ test_export_space_no_verify_ssl
…

C901, &#x27;import_content&#x27; is too complex (23)
E231, missing whitespace after &#x27;,&#x27;
E261, at least two spaces before inline comment
E262, inline comment should start with &#x27;# &#x27;
E265, block comment should start with &#x27;# &#x27;
E302, expected 2 blank lines, found 1
E303, too many blank lines (2)
W293, blank line contains whitespace
W391, blank line at end of file
pytest ‑ internal

github-actions · 2023-02-17T17:59:25Z

Python Tests

    6 files ±  0 168 suites ±0 5s ⏱️ +3s
  57 tests - 30   25 ✔️ -   62 0 💤 ±0   31 ❌ +  31 1 🔥 +1
462 runs +39 231 ✔️ - 192 0 💤 ±0 228 ❌ +228 3 🔥 +3

For more details on these failures and errors, see this check.

Results for commit 66a69a6. ± Comparison against base commit 1d080c2.

This pull request removes 40 and adds 10 tests. Note that renamed tests count towards both.

tests.test_deploy_content ‑ test_build_spaces
tests.test_deploy_content ‑ test_deploy_content_build_call
tests.test_deploy_content ‑ test_deploy_content_import_content_call
tests.test_deploy_content ‑ test_deploy_space_build_call
tests.test_deploy_content ‑ test_deploy_space_dashboard_call
tests.test_deploy_content ‑ test_deploy_space_look_call
tests.test_deploy_content ‑ test_import_content_win
tests.test_deploy_content_export ‑ test_export_space
tests.test_deploy_content_export ‑ test_export_space_debug
tests.test_deploy_content_export ‑ test_export_space_no_verify_ssl
…

C901, &#x27;import_content&#x27; is too complex (24)
E231, missing whitespace after &#x27;,&#x27;
E261, at least two spaces before inline comment
E262, inline comment should start with &#x27;# &#x27;
E265, block comment should start with &#x27;# &#x27;
E302, expected 2 blank lines, found 1
E303, too many blank lines (2)
W293, blank line contains whitespace
W391, blank line at end of file
pytest ‑ internal

jkaster · 2023-02-21T19:18:16Z

looker_deployer/commands/deploy_boards.py

@@ -13,7 +13,7 @@
 # limitations under the License.

 import logging
-from looker_sdk import models
+from looker_sdk import models40 as models


This will be good until the python SDK is updated after 3.1 is removed. Then models40 will probably be called models. We should discuss/research how much code this would break ... or maybe we alias models for models40. Worth thinking about @christelilaka @drstrangelooker et al

github-actions · 2023-02-22T21:55:15Z

Python Tests

    6 files ±  0 168 suites ±0 5s ⏱️ +3s
  57 tests - 30   25 ✔️ -   62 0 💤 ±0   31 ❌ +  31 1 🔥 +1
462 runs +39 231 ✔️ - 192 0 💤 ±0 228 ❌ +228 3 🔥 +3

For more details on these failures and errors, see this check.

Results for commit af83654. ± Comparison against base commit 1d080c2.

This pull request removes 40 and adds 10 tests. Note that renamed tests count towards both.

tests.test_deploy_content ‑ test_build_spaces
tests.test_deploy_content ‑ test_deploy_content_build_call
tests.test_deploy_content ‑ test_deploy_content_import_content_call
tests.test_deploy_content ‑ test_deploy_space_build_call
tests.test_deploy_content ‑ test_deploy_space_dashboard_call
tests.test_deploy_content ‑ test_deploy_space_look_call
tests.test_deploy_content ‑ test_import_content_win
tests.test_deploy_content_export ‑ test_export_space
tests.test_deploy_content_export ‑ test_export_space_debug
tests.test_deploy_content_export ‑ test_export_space_no_verify_ssl
…

C901, &#x27;import_content&#x27; is too complex (24)
E231, missing whitespace after &#x27;,&#x27;
E261, at least two spaces before inline comment
E262, inline comment should start with &#x27;# &#x27;
E265, block comment should start with &#x27;# &#x27;
E302, expected 2 blank lines, found 1
E303, too many blank lines (2)
W293, blank line contains whitespace
W391, blank line at end of file
pytest ‑ internal

github-actions · 2023-03-06T16:09:21Z

Python Tests

    6 files ±  0 168 suites ±0 5s ⏱️ +3s
  57 tests - 30   25 ✔️ -   62 0 💤 ±0   31 ❌ +  31 1 🔥 +1
462 runs +39 231 ✔️ - 192 0 💤 ±0 228 ❌ +228 3 🔥 +3

For more details on these failures and errors, see this check.

Results for commit 532f33e. ± Comparison against base commit 1d080c2.

This pull request removes 40 and adds 10 tests. Note that renamed tests count towards both.

tests.test_deploy_content ‑ test_build_spaces
tests.test_deploy_content ‑ test_deploy_content_build_call
tests.test_deploy_content ‑ test_deploy_content_import_content_call
tests.test_deploy_content ‑ test_deploy_space_build_call
tests.test_deploy_content ‑ test_deploy_space_dashboard_call
tests.test_deploy_content ‑ test_deploy_space_look_call
tests.test_deploy_content ‑ test_import_content_win
tests.test_deploy_content_export ‑ test_export_space
tests.test_deploy_content_export ‑ test_export_space_debug
tests.test_deploy_content_export ‑ test_export_space_no_verify_ssl
…

C901, &#x27;import_content&#x27; is too complex (26)
E231, missing whitespace after &#x27;,&#x27;
E261, at least two spaces before inline comment
E262, inline comment should start with &#x27;# &#x27;
E265, block comment should start with &#x27;# &#x27;
E302, expected 2 blank lines, found 1
E303, too many blank lines (2)
W293, blank line contains whitespace
W391, blank line at end of file
pytest ‑ internal

github-actions · 2023-06-26T17:38:43Z

Python Tests

    5 files -   1 167 suites - 1 2s ⏱️ -1s
  57 tests - 32   44 ✔️ -   45 0 💤 ±0   12 ❌ +  12 1 🔥 +1
415 runs - 14 245 ✔️ - 184 0 💤 ±0 168 ❌ +168 2 🔥 +2

For more details on these failures and errors, see this check.

Results for commit 7135a21. ± Comparison against base commit 7323c77.

This pull request removes 42 and adds 10 tests. Note that renamed tests count towards both.

tests.test_deploy_content ‑ test_build_spaces
tests.test_deploy_content ‑ test_deploy_content_build_call
tests.test_deploy_content ‑ test_deploy_content_import_content_call
tests.test_deploy_content ‑ test_deploy_space_build_call
tests.test_deploy_content ‑ test_deploy_space_dashboard_call
tests.test_deploy_content ‑ test_deploy_space_look_call
tests.test_deploy_content ‑ test_import_content_win
tests.test_deploy_content_export ‑ test_export_dashboard
tests.test_deploy_content_export ‑ test_export_look
tests.test_deploy_content_export ‑ test_export_space
…

C901, &#x27;import_content&#x27; is too complex (26)
E231, missing whitespace after &#x27;,&#x27;
E261, at least two spaces before inline comment
E262, inline comment should start with &#x27;# &#x27;
E265, block comment should start with &#x27;# &#x27;
E302, expected 2 blank lines, found 1
E303, too many blank lines (2)
W293, blank line contains whitespace
W391, blank line at end of file
pytest ‑ internal

adding alerts functionality, update to 40sdk

809234e

michaelcramer requested review from vpoiesz and AdamMinton October 6, 2022 15:06

michaelcramer changed the title ~~adding alerts functionality, update to 40sdk~~ feat: preserve alerts, update to 40sdk Oct 6, 2022

Merge remote-tracking branch 'origin/main' into mcramer_alerts

82d18a9

AdamMinton suggested changes Nov 4, 2022

View reviewed changes

michaelcramer added 5 commits December 2, 2022 08:53

all owners for alert search

ee4b846

str cast for matching

bf4e18c

add log for alert removal

42611df

correctly preserve owner

dbe11c8

update alert removal info

fccc0ac

use update_alert_field not update_alert

4d772c4

michaelcramer added 2 commits January 11, 2023 11:13

remove old alerts when creating new

b2d7955

fix for null to none filter

17c4d4a

fix error email when deleting alerts

66a69a6

jkaster reviewed Feb 21, 2023

View reviewed changes

alerts unfollowable fix

af83654

michaelcramer and others added 2 commits March 6, 2023 08:09

potential fix for error emails

532f33e

Merge branch 'main' into mcramer_alerts

7135a21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: preserve alerts, update to 40sdk #97

feat: preserve alerts, update to 40sdk #97

michaelcramer commented Oct 6, 2022

github-actions bot commented Oct 6, 2022

github-actions bot commented Oct 7, 2022

AdamMinton left a comment

AdamMinton Nov 4, 2022

AdamMinton Nov 4, 2022

AdamMinton Nov 4, 2022

AdamMinton Nov 4, 2022

AdamMinton Nov 4, 2022

AdamMinton Nov 4, 2022

AdamMinton Nov 4, 2022

AdamMinton Nov 4, 2022

AdamMinton Nov 4, 2022

AdamMinton Nov 4, 2022

github-actions bot commented Jan 10, 2023

github-actions bot commented Jan 11, 2023

github-actions bot commented Feb 15, 2023

github-actions bot commented Feb 17, 2023

jkaster Feb 21, 2023

github-actions bot commented Feb 22, 2023

github-actions bot commented Mar 6, 2023

github-actions bot commented Jun 26, 2023

feat: preserve alerts, update to 40sdk #97

Are you sure you want to change the base?

feat: preserve alerts, update to 40sdk #97

Conversation

michaelcramer commented Oct 6, 2022

github-actions bot commented Oct 6, 2022

Python Tests

github-actions bot commented Oct 7, 2022

Python Tests

AdamMinton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 10, 2023

Python Tests

github-actions bot commented Jan 11, 2023

Python Tests

github-actions bot commented Feb 15, 2023

Python Tests

github-actions bot commented Feb 17, 2023

Python Tests

Choose a reason for hiding this comment

github-actions bot commented Feb 22, 2023

Python Tests

github-actions bot commented Mar 6, 2023

Python Tests

github-actions bot commented Jun 26, 2023

Python Tests