-
Notifications
You must be signed in to change notification settings - Fork 25
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
feat: preserve alerts, update to 40sdk #97
base: main
Are you sure you want to change the base?
Changes from all commits
809234e
82d18a9
ee4b846
bf4e18c
42611df
dbe11c8
fccc0ac
4d772c4
b2d7955
17c4d4a
66a69a6
af83654
532f33e
7135a21
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ | |
import tempfile | ||
import shutil | ||
import threading | ||
import json | ||
from concurrent.futures import ThreadPoolExecutor | ||
from itertools import repeat | ||
from looker_deployer.utils import deploy_logging | ||
|
@@ -29,6 +30,14 @@ | |
|
||
logger = deploy_logging.get_logger(__name__) | ||
|
||
def alert_cleanup(sdk): | ||
logger.debug("cleaning up orphaned alerts") | ||
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']) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also logger.info for any time we do delete or updating so people can see which IDs changed |
||
logger.info("Alert removed", extra={"alert title": alert['custom_title'], "owner": alert['owner_display_name']}) | ||
|
||
|
||
def get_space_ids_from_name(space_name, parent_id, sdk): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
if (space_name == "Shared" and parent_id == "0"): | ||
|
@@ -125,8 +134,76 @@ def import_content(content_type, content_json, space_id, env, ini, debug=False): | |
win_exec = ["cmd.exe", "/c"] | ||
gzr_command = win_exec + gzr_command | ||
|
||
is_new_dash = "false" | ||
existing_dash_alerts = [] | ||
existing_elements_w_alerts = [] | ||
|
||
if content_type == "dashboard": ## only run for dashboards, looks can't have alerts | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible to move this into a separate function that is called from here to allow for unit tests? |
||
sdk = get_client(ini, env) ## should we update the function def and pass sdk? | ||
with open(content_json) as file: | ||
json_dash = json.load(file) | ||
existing_dash = sdk.search_dashboards(slug=json_dash['slug']) ## search with slug first, fall back to name + folder | ||
if len(existing_dash) < 1: | ||
existing_dash = sdk.search_dashboards(title=json_dash['title'],folder_id=space_id) | ||
if len(existing_dash) < 1: | ||
is_new_dash = "true" | ||
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: str(alert['dashboard_element_id']) == str(element['id']), enabled_alerts)) | ||
if len(alerts) > 0: | ||
existing_dash_alerts.extend(alerts) | ||
if len(existing_dash_alerts) > start: | ||
if element not in existing_elements_w_alerts: | ||
existing_elements_w_alerts.append(element) | ||
|
||
|
||
#logger.debug("space info", extra={"space_name": space_name, "parent_id": parent_id}) | ||
update_fields = {} | ||
update_fields['owner_id'] = sdk.me()['id'] #get id of current user | ||
update_fields['is_disabled'] = "true" | ||
update_fields['disabled_reason'] = "dashboard update" | ||
if len(existing_dash_alerts) > 0: | ||
for alert in existing_dash_alerts: | ||
if alert['id'] != update_fields['owner_id']: | ||
sdk.update_alert_field(alert['id'], update_fields) #update old alert to current user to prevent errors when deleting | ||
|
||
subprocess.run(gzr_command) | ||
|
||
|
||
if is_new_dash == "false" and content_type == "dashboard" and len(existing_dash_alerts) > 0: ## get the new dashboard | ||
updated_dash = sdk.search_dashboards(slug=json_dash['slug']) | ||
if len(updated_dash) < 1: | ||
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'])) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
if len(updated_dash_element) == 1: ## what should we do if more than one match? | ||
old_to_new_ids[element['id']] = updated_dash_element[0]['id'] | ||
for alert in existing_dash_alerts: ## create new alerts | ||
logger.debug('processing alert for element', extra={"element_id": alert['dashboard_element_id']}) | ||
new_alert = {} | ||
update_owner = {} #alerts are assigned to creator, need to update after | ||
update_owner['owner_id'] = alert['owner_id'] | ||
for key in alert.keys(): | ||
new_alert[key] = alert[key] | ||
new_alert['applied_dashboard_filters'] = [] | ||
new_filter = {} | ||
for old_filter in alert['applied_dashboard_filters']: | ||
new_filter = old_filter.__dict__ | ||
if new_filter['filter_value'] == 'None': | ||
new_filter['filter_value'] = "" | ||
new_alert['applied_dashboard_filters'].append(new_filter) | ||
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']]}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we get an logger.info line that summarizes what is happening that specifies the dashboard element ID and alert being updated. |
||
new_alert['dashboard_element_id'] = old_to_new_ids[alert['dashboard_element_id']] | ||
try: | ||
created_alert = sdk.create_alert(new_alert) | ||
sdk.update_alert_field(created_alert['id'], update_owner) #update new alert to correct owner | ||
sdk.delete_alert(alert['id']) | ||
except Exception as e: | ||
print(e) | ||
|
||
|
||
def build_spaces(spaces, sdk): | ||
# seeding initial value of parent id to Shared | ||
|
@@ -303,6 +380,8 @@ def main(args): | |
args.target_base = 'Shared' | ||
|
||
sdk = get_client(args.ini, args.env) | ||
global enabled_alerts | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we can get an update to the readme about this new functionality |
||
enabled_alerts = sdk.search_alerts(disabled="false", all_owners=True) | ||
send_content( | ||
sdk, | ||
args.env, | ||
|
@@ -315,3 +394,6 @@ def main(args): | |
args.debug, | ||
args.target_base | ||
) | ||
|
||
alert_cleanup(sdk) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this doesn't work for me, my alerts are being unset but not disabled |
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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