Skip to content

Commit

Permalink
Address deploy-board lint errors (#1643)
Browse files Browse the repository at this point in the history
`ruff` was recently introduced to the precommit checks.

This PR addresses the existing checks for deploy-board:

* Run `ruff check --fix`
* Run `ruff check --fix --unsafe-fixes`
  * Verified the fixes were safe
* Use explicit exceptions
* Remove unused variables and methods
* Remove trailing ws
  • Loading branch information
osoriano authored Jun 3, 2024
1 parent 7f61746 commit d9faf94
Show file tree
Hide file tree
Showing 28 changed files with 112 additions and 118 deletions.
7 changes: 4 additions & 3 deletions deploy-board/deploy_board/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#
# http://www.apache.org/licenses/LICENSE-2.0
#
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -18,9 +18,10 @@
from django.conf.urls.static import static
from django.conf import settings

from .settings import IS_PINTEREST

admin.autodiscover()

from .settings import IS_PINTEREST

if IS_PINTEREST:
urlpatterns = [
Expand Down
2 changes: 1 addition & 1 deletion deploy-board/deploy_board/webapp/accounts.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def get_accounts(report):

def add_account_from_cluster(request, cluster, accounts):
account_id = cluster.get("accountId")
account = None;
account = None
if account_id is not None:
account = accounts_helper.get_by_cell_and_id(
request, cluster["cellName"], account_id)
Expand Down
2 changes: 1 addition & 1 deletion deploy-board/deploy_board/webapp/accounts_views.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from django.shortcuts import render, redirect
from django.views.generic import View

from .helpers import accounts_helper;
from .helpers import accounts_helper

class AccountsView(View):
def get(self, request):
Expand Down
2 changes: 0 additions & 2 deletions deploy-board/deploy_board/webapp/agent_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@
# -*- coding: utf-8 -*-
"""Helper functions to help generate agents views
"""
from deploy_board.webapp.helpers import agents_helper
from .common import is_agent_failed
from .helpers import builds_helper, deploys_helper, environs_helper, environ_hosts_helper
from deploy_board.settings import IS_PINTEREST
import time
from collections import OrderedDict
from functools import cmp_to_key
Expand Down
16 changes: 8 additions & 8 deletions deploy-board/deploy_board/webapp/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,25 +82,25 @@ class OAuthExpiredTokenException(Exception):
class OAuthHandler(object):

def token_getter(self, **kwargs):
raise NotImplemented
raise NotImplementedError

def token_setter(self, token, expires, **kwargs):
raise NotImplemented
raise NotImplementedError

def state_generator(self, **kwargs):
raise NotImplemented
raise NotImplementedError

def state_getter(self, **kwargs):
raise NotImplemented
raise NotImplementedError

def state_setter(self, value, **kwargs):
raise NotImplemented
raise NotImplementedError

def state_remove(self, **kwargs):
raise NotImplemented
raise NotImplementedError

def token_remove(self, **kwargs):
raise NotImplemented
raise NotImplementedError


class SessionOauthHandler(OAuthHandler):
Expand Down Expand Up @@ -210,7 +210,7 @@ def handle_oauth2_response(self, code, state_with_data, **kwargs):
data=str(body) if body else None,
method='POST',
)
if resp.code is 401:
if resp.code == 401:
# When auth.pinadmin.com returns a 401 error. remove token and redirect to / page
raise OAuthExpiredTokenException("Expired Token")

Expand Down
20 changes: 11 additions & 9 deletions deploy-board/deploy_board/webapp/cluster_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

from .helpers import baseimages_helper, hosttypes_helper, securityzones_helper, placements_helper, \
autoscaling_groups_helper, groups_helper, cells_helper, arches_helper, accounts_helper
from .helpers import clusters_helper, environs_helper, environ_hosts_helper, baseimages_helper
from .helpers import clusters_helper, environs_helper, environ_hosts_helper
from .helpers.exceptions import NotAuthorizedException, TeletraanException, IllegalArgumentException
from . import common
import traceback
Expand Down Expand Up @@ -197,6 +197,7 @@ def get(self, request, name, stage):

def post(self, request, name, stage):
ret = 200
exception = None
log.info("Post to capacity with data {0}".format(request.body))
try:
cluster_name = '{}-{}'.format(name, stage)
Expand All @@ -218,16 +219,18 @@ def post(self, request, name, stage):
except NotAuthorizedException as e:
log.error("Have an NotAuthorizedException error {}".format(e))
ret = 403
exception = e
except Exception as e:
log.error("Have an error {}", e)
ret = 500
exception = e
finally:
if ret == 200:
return HttpResponse("{}", content_type="application/json")
else:
environs_helper.remove_env_capacity(
request, name, stage, capacity_type="GROUP", data=cluster_name)
return HttpResponse(e, status=ret, content_type="application/json")
return HttpResponse(exception, status=ret, content_type="application/json")


class ClusterConfigurationView(View):
Expand Down Expand Up @@ -1137,13 +1140,13 @@ def gen_auto_cluster_refresh_view(request, name, stage):

# get default configurations for first time
try:
if auto_refresh_config == None:
if auto_refresh_config is None:
auto_refresh_config = clusters_helper.get_default_cluster_auto_refresh_config(request, cluster_name)
auto_refresh_config["launchBeforeTerminate"] = True
auto_refresh_config["config"]["minHealthyPercentage"] = 100
auto_refresh_config["config"]["maxHealthyPercentage"] = 110
else:
if auto_refresh_config["config"]["maxHealthyPercentage"] == None:
if auto_refresh_config["config"]["maxHealthyPercentage"] is None:
auto_refresh_config["terminateAndLaunch"] = True
elif auto_refresh_config["config"]["minHealthyPercentage"] == 100:
auto_refresh_config["launchBeforeTerminate"] = True
Expand Down Expand Up @@ -1179,7 +1182,7 @@ def gen_auto_cluster_refresh_view(request, name, stage):

def sanitize_slack_email_input(input):
res = ''
if input == None or len(input) == 0:
if input is None or len(input) == 0:
return res

tokens = input.strip().split(',')
Expand Down Expand Up @@ -1264,7 +1267,7 @@ def submit_auto_refresh_config(request, name, stage):
group_info["groupInfo"]["chatroom"] = slack_channels
autoscaling_groups_helper.update_group_info(request, cluster_name, group_info["groupInfo"])
messages.success(request, "Auto refresh config saved successfully.", "cluster-replacements")
except IllegalArgumentException as e:
except IllegalArgumentException:
log.exception("Failed to update refresh config. Some request could succeed.")
pass
except Exception as e:
Expand All @@ -1290,13 +1293,13 @@ def gen_replacement_config(request):

if params["availabilitySettingRadio"] == "launchBeforeTerminate":
rollingUpdateConfig["minHealthyPercentage"] = 100
if params["maxHealthyPercentage"] == None or len(params["maxHealthyPercentage"]) == 0:
if params["maxHealthyPercentage"] is None or len(params["maxHealthyPercentage"]) == 0:
rollingUpdateConfig["maxHealthyPercentage"] = 110
else:
rollingUpdateConfig["maxHealthyPercentage"] = params["maxHealthyPercentage"]
elif params["availabilitySettingRadio"] == "terminateAndLaunch":
rollingUpdateConfig["maxHealthyPercentage"] = None
if params["minHealthyPercentage"] == None or len(params["minHealthyPercentage"]) == 0:
if params["minHealthyPercentage"] is None or len(params["minHealthyPercentage"]) == 0:
rollingUpdateConfig["minHealthyPercentage"] = 100
else:
rollingUpdateConfig["minHealthyPercentage"] = params["minHealthyPercentage"]
Expand Down Expand Up @@ -1502,4 +1505,3 @@ def get_current_cluster(request, name, stage, env=None):
cluster_name = env.get("clusterName")
current_cluster = clusters_helper.get_cluster(request, cluster_name)
return current_cluster

1 change: 0 additions & 1 deletion deploy-board/deploy_board/webapp/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
"""
import logging
from .helpers import environs_helper, deploys_helper, builds_helper, tags_helper
from deploy_board.settings import IS_PINTEREST

DEFAULT_BUILD_SIZE = 30
DEFAULT_COMMITS_SIZE = 30
Expand Down
6 changes: 4 additions & 2 deletions deploy-board/deploy_board/webapp/deploy_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# -*- coding: utf-8 -*-
"""Collection of all deploy related views
"""
import logging
from deploy_board.settings import SITE_METRICS_CONFIGS, TELETRAAN_DISABLE_CREATE_ENV_PAGE, TELETRAAN_REDIRECT_CREATE_ENV_PAGE_URL
from django.middleware.csrf import get_token
from .accounts import get_accounts_from_deploy
Expand All @@ -23,9 +24,11 @@
from django.views.generic import View
from django.template.loader import render_to_string
from django.http import HttpResponse
from .helpers import builds_helper, deploys_helper, environs_helper, tags_helper, clusters_helper, accounts_helper
from .helpers import builds_helper, deploys_helper, environs_helper, tags_helper


log = logging.getLogger(__name__)

DEFAULT_PAGE_SIZE = 30
DEFAULT_ONGOING_DEPLOY_SIZE = 10

Expand Down Expand Up @@ -145,7 +148,6 @@ def get(self, request, deploy_id):
deploy = deploys_helper.get(request, deploy_id)
build_with_tag = builds_helper.get_build_and_tag(request, deploy['buildId'])
env = None
account = None
deploy_accounts = []
if deploy.get('envId'):
env = environs_helper.get(request, deploy['envId'])
Expand Down
21 changes: 9 additions & 12 deletions deploy-board/deploy_board/webapp/env_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
from .accounts import get_accounts, get_accounts_from_deploy, create_legacy_ui_account, add_account_from_cluster, add_legacy_accounts
import random
import json
import requests
from collections import Counter
from .helpers import builds_helper, environs_helper, agents_helper, ratings_helper, deploys_helper, \
systems_helper, environ_hosts_helper, clusters_helper, tags_helper, baseimages_helper, schedules_helper, placements_helper, hosttypes_helper, \
Expand Down Expand Up @@ -356,7 +355,7 @@ def check_feedback_eligible(request, username):
if num <= 10:
return True
return False
except:
except Exception:
log.error(traceback.format_exc())
return False

Expand Down Expand Up @@ -605,13 +604,13 @@ def _gen_message_for_refreshing_cluster(request, last_cluster_refresh_status, la

any_host_with_outdated_ami = len(amis) > 1 or (len(amis) == 1 and amis[0] != current_AMI)

if any_host_with_outdated_ami and latest_succeeded_base_image_update_event != None:
if last_cluster_refresh_status == None or last_cluster_refresh_status["startTime"] == None or last_cluster_refresh_status["startTime"] <= latest_succeeded_base_image_update_event["finish_time"]:
if any_host_with_outdated_ami and latest_succeeded_base_image_update_event is not None:
if last_cluster_refresh_status is None or last_cluster_refresh_status["startTime"] is None or last_cluster_refresh_status["startTime"] <= latest_succeeded_base_image_update_event["finish_time"]:
return "The cluster was updated with a new AMI at {} PST and should be replaced to ensure the AMI is applied to all existing hosts.".format(utils.convertTimestamp(latest_succeeded_base_image_update_event["finish_time"]))

return None

except:
except Exception:
# in case of any exception, return None instead of showing the error on landing page
return None

Expand All @@ -626,7 +625,7 @@ def _get_last_cluster_refresh_status(request, env):
return None

return replace_summaries["clusterRollingUpdateStatuses"][0]
except:
except Exception:
return None

def _is_cluster_auto_refresh_enabled(request, env):
Expand All @@ -635,7 +634,7 @@ def _is_cluster_auto_refresh_enabled(request, env):
cluster_info = clusters_helper.get_cluster(request, cluster_name)

return cluster_info["autoRefresh"]
except:
except Exception:
return None


Expand Down Expand Up @@ -689,7 +688,7 @@ def _get_commit_info(request, commit, repo=None, branch='master'):
try:
commit_info = builds_helper.get_commit(request, repo, commit)
return repo, branch, commit_info['date']
except:
except Exception:
log.error(traceback.format_exc())
return None, None, None

Expand Down Expand Up @@ -1024,7 +1023,7 @@ def post_create_env(request):
if external_id:
try:
environs_helper.delete_nimbus_identifier(request, external_id)
except:
except Exception:
message = 'Also failed to delete Nimbus identifier {}. Please verify that identifier no longer exists, Error Message: {}'.format(external_id, detail)
log.error(message)
raise detail
Expand Down Expand Up @@ -1322,7 +1321,6 @@ def rollback(request, name, stage):
html = render_to_string("environs/env_rollback.html", {
"envs": envs,
"stages": stages,
"envs": envs,
"env": env,
"deploy_summaries": deploy_summaries,
"to_deploy_id": to_deploy_id,
Expand Down Expand Up @@ -1368,7 +1366,6 @@ def promote(request, name, stage, deploy_id):
html = render_to_string("environs/env_promote.html", {
"envs": envs,
"stages": stages,
"envs": envs,
"env": env,
"env_wrappers": env_wrappers,
"deploy": deploy,
Expand Down Expand Up @@ -1806,7 +1803,7 @@ def get_deploy_schedule(request, name, stage):
env = environs_helper.get_env_by_stage(request, name, stage)
envs = environs_helper.get_all_env_stages(request, name)
schedule_id = env.get('scheduleId', None)
if schedule_id != None:
if schedule_id is not None:
schedule = schedules_helper.get_schedule(request, name, stage, schedule_id)
else:
schedule = None
Expand Down
2 changes: 1 addition & 1 deletion deploy-board/deploy_board/webapp/error_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from django.shortcuts import render
from deploy_board.settings import DEBUG
from django.http import HttpResponse, HttpResponseRedirect
from .helpers.exceptions import NotAuthorizedException, FailedAuthenticationException
from .helpers.exceptions import IllegalArgumentException, NotAuthorizedException, NotFoundException, FailedAuthenticationException

logger = logging.getLogger(__name__)

Expand Down
Loading

0 comments on commit d9faf94

Please sign in to comment.