From 9e175e96c3e8ac5fad0dd666d0b0a513fe2dbf28 Mon Sep 17 00:00:00 2001 From: Alan Yee Date: Mon, 7 Oct 2019 09:15:15 -0700 Subject: [PATCH] Pythonic changes (#51) * Update constants.py Constants should not have operations within it * Update minion_manager.py * Update asg_mm.py -Constants should not have unnecessary operators -.get() returns None by default if value is missing -Other optimizations * Update aws_bid_advisor.py array itself over len() --- cloud_provider/aws/asg_mm.py | 27 ++++++++++----------------- cloud_provider/aws/aws_bid_advisor.py | 8 +++----- constants.py | 2 +- minion_manager.py | 5 +++-- 4 files changed, 17 insertions(+), 25 deletions(-) diff --git a/cloud_provider/aws/asg_mm.py b/cloud_provider/aws/asg_mm.py index e9fb74e..cf808aa 100644 --- a/cloud_provider/aws/asg_mm.py +++ b/cloud_provider/aws/asg_mm.py @@ -1,12 +1,10 @@ """ Metadata for each Autoscaling group in AWS. """ MINION_MANAGER_LABEL = 'k8s-minion-manager' -NOT_TERMINATE_LABEL = MINION_MANAGER_LABEL+'/not-terminate' +NOT_TERMINATE_LABEL = 'k8s-minion-manager/not-terminate' class AWSAutoscalinGroupMM(object): - """ - This class has metadata associated with an autoscaling group. - """ + """This class has metadata associated with an autoscaling group.""" def __init__(self): # 'asg_info' is populated with the returned value of @@ -32,11 +30,11 @@ def set_asg_info(self, asg_info): assert asg_info is not None, "Can't set ASG info to None!" self.asg_info = asg_info for tag in self.asg_info['Tags']: - if tag.get('Key', None) == MINION_MANAGER_LABEL: + if tag.get('Key') == MINION_MANAGER_LABEL: if tag['Value'] not in ("use-spot", "no-spot"): tag['Value'] = 'no-spot' - elif tag.get('Key', None) == NOT_TERMINATE_LABEL: - if tag['Value'] in ('true', 'True', 'TRUE'): + elif tag.get('Key') == NOT_TERMINATE_LABEL: + if tag['Value'].lower() == 'true': tag['Value'] = True else: tag['Value'] = False @@ -64,20 +62,15 @@ def get_bid_info(self): return self.bid_info def add_instances(self, instances): - """ - Adds the given instances to the 'instances' array if they're not + """Adds the given instances to the 'instances' array if they're not present. """ for instance in instances: - if instance.InstanceId in self.instance_info.keys(): - continue - else: + if instance.InstanceId not in self.instance_info: self.instance_info[instance.InstanceId] = instance def remove_instance(self, instance_id): - """ - Removes the given instance from the 'instances' array. - """ + """Removes the given instance from the 'instances' array.""" self.instance_info.pop(instance_id, None) def get_instance_info(self): @@ -90,7 +83,7 @@ def get_instances(self): def get_mm_tag(self): for tag in self.asg_info['Tags']: - if tag.get('Key', None) == MINION_MANAGER_LABEL: + if tag.get('Key') == MINION_MANAGER_LABEL: return tag['Value'].lower() return "no-spot" @@ -119,6 +112,6 @@ def is_instance_running(self, instance): def not_terminate_instance(self): """ Retures is the ASG set not terminate instance """ for tag in self.asg_info['Tags']: - if tag.get('Key', None) == NOT_TERMINATE_LABEL: + if tag.get('Key') == NOT_TERMINATE_LABEL: return tag['Value'] return False diff --git a/cloud_provider/aws/aws_bid_advisor.py b/cloud_provider/aws/aws_bid_advisor.py index 3d10192..a6ccb92 100644 --- a/cloud_provider/aws/aws_bid_advisor.py +++ b/cloud_provider/aws/aws_bid_advisor.py @@ -209,7 +209,7 @@ def run(self): def run(self): """ Main method of the AWSBidAdvisor. """ - if len(self.all_bid_advisor_threads) > 0: + if self.all_bid_advisor_threads: logger.debug("BidAdvisor already running!") return @@ -234,8 +234,7 @@ def run(self): logger.info("Waiting for initial pricing information...") try: with self.lock: - if len(self.on_demand_price_dict) != 0 and \ - len(self.spot_price_list) != 0: + if self.on_demand_price_dict and self.spot_price_list: return finally: time.sleep(SECONDS_PER_MINUTE) @@ -326,8 +325,7 @@ def get_new_bid(self, zones, instance_type): """ with self.lock: - if len(self.on_demand_price_dict) == 0 or \ - len(self.spot_price_list) == 0: + if not self.on_demand_price_dict or not self.spot_price_list: logger.info("Pricing data not available! Using DEFAULT_BID") return DEFAULT_BID diff --git a/constants.py b/constants.py index 274a282..ad44d1e 100644 --- a/constants.py +++ b/constants.py @@ -1,4 +1,4 @@ """Constants used by the minion-manager.""" SECONDS_PER_MINUTE = 60 -SECONDS_PER_HOUR = (60 * 60) \ No newline at end of file +SECONDS_PER_HOUR = 3600 diff --git a/minion_manager.py b/minion_manager.py index a013b97..f27ef9f 100644 --- a/minion_manager.py +++ b/minion_manager.py @@ -20,7 +20,7 @@ def validate_usr_args(usr_args): Validates the arguments provided by the user. """ assert usr_args.cloud.lower() == "aws", "Only AWS is currently supported." - assert usr_args.cluster_name != None, "Cluster name is required" + assert usr_args.cluster_name is not None, "Cluster name is required" if "profile" not in usr_args: usr_args.profile = None @@ -39,7 +39,8 @@ def run(): parser.add_argument("--profile", help="Credentials profile to use") parser.add_argument("--refresh-interval-seconds", default="300", help="Interval in seconds at which to query AWS") - parser.add_argument("--cluster-name", required=True, help="Name of the Kubernetes cluster. Get's used for identifying ASGs") + parser.add_argument("--cluster-name", required=True, + help="Name of the Kubernetes cluster. Get's used for identifying ASGs") usr_args = parser.parse_args() validate_usr_args(usr_args)