code refactoring, ebs volume extension for testnet#150
code refactoring, ebs volume extension for testnet#150AndyBoWu wants to merge 4 commits intoharmony-one:masterfrom
Conversation
LeoHChen
left a comment
LeoHChen
left a comment
There was a problem hiding this comment.
initial review. needs further optimization to reduce redundancy.
also, is this script only working for testnet? how about mainnet? why can't we use the same script and different parameter?
| import botocore | ||
| from botocore.exceptions import ClientError | ||
|
|
||
| EBS_size = 40 |
| s3_client = boto3.resource('s3') | ||
| s3_client.Bucket(bucketname).download_file(remote_file, local_file) | ||
|
|
||
| def create_ip_for_each_region(fpath): |
There was a problem hiding this comment.
use of this function for what? no explanation.
There was a problem hiding this comment.
is it just parsing for distribution_config file? is there any other code can be reuse?
| while line: | ||
| if line.rstrip(): | ||
| ip = line.split()[0] | ||
| region_num = line.split()[4][0] |
There was a problem hiding this comment.
what's the format of original file, how do you parse it?
|
|
||
| def create_ebs_volume_id_each_region(dict_ip): | ||
| for region, ip_array in dict_ip.items(): | ||
| if region == '1': |
There was a problem hiding this comment.
why duplicate the code for different region? this is not efficient. Just add a map of region and region_name.
|
|
||
| def increase_ebs_volume(dict_ebs): | ||
| for region, ebs_id_array in dict_ebs.items(): | ||
| if region == '1': |
There was a problem hiding this comment.
same thing, please don't duplicate code.
| dict_region_volid_array = {} | ||
|
|
||
| load_dotenv() | ||
| GIT_TOKEN = os.getenv('GIT_TOKEN') |
There was a problem hiding this comment.
this can possibly fail, perhaps better to call it during usage
| load_dotenv() | ||
| GIT_TOKEN = os.getenv('GIT_TOKEN') | ||
|
|
||
| profile_array = ['default', 'mainnet-aw'] |
There was a problem hiding this comment.
constants all upper case
| def shcmd(cmd, ignore_error=False): | ||
| ret = subprocess.call(cmd, shell=True) | ||
| if ignore_error == False and ret != 0: | ||
| raise RuntimeError("Failed to execute {}. Return code:{}".format( |
There was a problem hiding this comment.
prefer python f strings in new code
| return output_string | ||
|
|
||
| def download_ipfiles_github(ip_array): | ||
| for file in ip_array: |
There was a problem hiding this comment.
don't name your variable same name as python builtin names (file)
| # local_shard_file_array = [current_dir+"/shard0.txt"] | ||
| print(local_shard_file_array) | ||
| for item in local_shard_file_array: | ||
| fp = open(item) |
There was a problem hiding this comment.
prefer with open() as fp so with context handles the .close for you
| for item in local_shard_file_array: | ||
| fp = open(item) | ||
| line = fp.readline() | ||
| while line: |
There was a problem hiding this comment.
iirc there is a readlines() or read all, no need to do this on own
| region = 'us-east-1' | ||
| else: | ||
| raise ValueError("cannot deduce region from name {}".format(info)) | ||
| except Exception as e: |
There was a problem hiding this comment.
why catch exception at all in this case, maybe just let explode
| try: | ||
| resp = ec2.describe_instances(Filters=[{'Name': 'ip-address', 'Values': [ip]}]) | ||
| print(resp) | ||
| if len(resp["Reservations"]) != 0: |
There was a problem hiding this comment.
!= 0 not needed, just len(resp[])
AndyBoWu
force-pushed
the
pr_extend_ebs_testnet
branch
from
c08a9f2 to
07f0cfb
Compare
| local_dist_file = temp_dir + '/distribution_config.txt' | ||
|
|
||
| dict_ip_per_region = {} | ||
| dict_ebs_id_region = {} |
4 participants
first part to extend EBS volume to 40 GB for testnet