Tag Info

5

It's mostly fine, but I would suggest some minor improvements. Give a proper name to $1 early on, for example: host=$1 if [ -z $host ]; then echo "Please enter machine name" return 1 fi You use it in several places later and it will make the code more readable. Instead of this: if [ -z "${instance}" ]; then you can use [[ ... ]] instead and drop ...

5

Your code is a little hard to read, mostly due to you moving what should be an inner for loop, to be two ifs in an infinite while loop. If you were to instead for loop over pool, then you could make the code easier to understand. I'd also make page_size be a keyword argument, so the user can change the page size if they wish. And I'd change config to region,...

4

is there a reason for $(which foo) ? If foo is in your path, it will be found, if not which will give you an error. Expressions like command will have a return code, so do not write if [[$(command -v "$dependency") = "" ]] just use if command -v "$dependency" > /dev/null then ...

4

Syntax errors $aws_region = "--region$aws_region" This is not how you set a variable in Bash. The correct syntax is aws_region="--region $aws_region" Likewise for the later statements. Note that there are no spaces before and after the =, and you drop the sigil before the variable name when you're assigning to it. Use the appropriate shell parameter ... 4 Simplicity Tuple unpacking and str.format can simplify the function so much: def create_date_parition_from_key(key): a,b,c,_,year, month, day, _, name = key.split('/') return "{}/{}/{}/daily/dt={}-{}-{}/{}".format(\ a, b, c, year, month, day, name) If input key can change in the number of paths: def create_date_parition_from_key(key): ... 4 I think your function names could use some work. There is no need to specify that return_candidates_for_deletion actually returns those candidates (it is obvious). It is also a bit superfluous to know that extract_list_of_backups returns a list. It is enough to know that it returns a collection that is iterable (whether or not that is a tuple, list, set or ... 3$(which foo) ... First of all, I completely agree with the other answer, this is pointless, inefficient and ugly. Clearing variables Instead of var="", you can write simply: var= Exit code When a dependency is missing, the script does exit 0. Exit code 0 means success. It would make more sense to exit with non-zero, to indicate failure, for example exit ...

3

I haven't used much boto, so I can just focus on some style comments: Use four spaces per indentation level Remove unused imports (boto.exception) Try to order imports in alphabetical order Move argument parsing and logging configuration code to separate functions Be careful of the usage of next. What you were probably looking for is continue, but you can ...

3

you can use emr wait instead of describe-cluster aws emr wait cluster-running --cluster-id <value> --region <value> http://docs.aws.amazon.com/cli/latest/reference/emr/wait/cluster-running.html

3

First of all, splitting the spliterator only makes sense, if both the remainder of the current spliterator and the returned spliterator still have work pending. In your case, this is (almost) not true, as you operate on the complete batches and the current spliterator at most returns its current batch after split. Thus, I'd replace the trySplit() with a ...

3

There's nothing outstandingly wrong, there are some minor points that can be improved. The main concern that I have is the amount of logic happening inside the ProductRoutes file. Your invocation of the AWS library and the validate & saveImage functions can be stored inside reusable files that can be referenced anywhere. I normally would store the ...

3

Welcome to Code Review! Hopefully this experience will be useful and positive for you. Your statement about polling and then the function below basically polls my database every 1 minute looking for new rows that have a null value & performs actions on them until the lastUpdated can then be updated (once an action is performed elsewhere) I haven't ...

3

The S3-functionality wraps the specific code, which does not change the variables. So how about a decorator? from functools import wraps def handle_s3_upload(fcn): @wraps(fcn) def inner(data_fp, data, *args, **kwargs): # handle S3 path s3 = False if data_fp.lower().startswith('s3://'): s3_data_fp = data_fp ...

2

Your shebang works fine, but this idiom is more portable: #! /usr/bin/env bash You wrote TimeStamp: must be YYMMDD..., but it should say YYYY. Consider deleting the next fairly obvious line that says YY, or put YYYY there, as well. Please rephrase the comment like this: # Amazon Hosting URLs are built up in sections. The next comment line is extremely ...

2

When running the provided code on my laptop (Mint 19 / openssl 1.1.0g / bash 4.4.19(1)), it failed to generate a valid signature. This was the section causing problems: # Calculate the signature kDate=$(echo -n${date} | openssl dgst -sha256 -binary -hmac "AWS4${secret}") kRegn=$(echo -n ${region} | openssl dgst -sha256 -binary -hmac "${kDate}")...

2

The local variables make this very verbose: each variable is declared, initialized, re-assigned, and referenced. (They each appear 3-4 times in the code.) You could instead accumulate values in a list. Creating an array that may contain null values, then filtering out the null values in a second step feels like a waste that's easy to avoid. You could ...

2

After a quick read-through, I’ve spotted two items: with not used for f_out The code: with open(src) as f_in: f_out = gzip.open(destpath, 'wb') #... f_out.close() should be replaced with: with open(arc) as f_in, gzip.open(destpath, 'wb') as f_out: #... Reg-ex pattern repeatedly compiled The function regex_match() takes a string and ...

2

In general I would say the code looks fine as far as functionality, though there are a few specific points which I will mention below with more detail. The broadest critique I have is that there are some variables declared with var or let that could use const since they are never re-assigned - for instance validator, ext, params, request, rejected, accepted ...

2

aws_object = json.dumps({"S3Object":{"Bucket":self.s3_bucket,"Name":self.filename}}).replace(' ', '') # can't have any spaces lol picky AWS CLI start_textract_command = "aws textract start-document-text-detection --document-location '{}'".format(aws_object) This is not AWS which is picky, rather you’re not building the command line properly. If you provide ...

1

general The code looks good and readable, however I have some remarks. (I could not run the original code because of the use of 'f-string', a feature since Python 3.6 and I am still at 3.5) class not needed In this case a class is not needed, a simple function would satisfy. If you want to hide the helper functions validate_key and validate_value you ...

1

Your code looks very clean. I am not sure there is much to improve on, especially after your third improvement. It seems you are looking for millisecond, sub millisecond improvement. One area of improvement would be switch to a compiled language to be more performant. Check out the Latest performance stats by language for AWS Lambda.

1

Is there an actual way to profile Node.JS running in AWS Lambda to inspect the possible bottlenecks? Node Lambdas are just small Node modules. Profiling it shouldn't be any different than any other Node app. All you need is a "wrapper" app that imports your lambda script and calls handler with a fake event and context. Your biggest enemy here won't be your ...

1

I would like to know if it is possible to avoid all the if statements for error validations. No. This is the way errors are handled in Go. You get the error as return value and you handle it. You may return it, log it, print it or mute. You can panic with it, which is somewhat similar to exceptions. It's all up to you. Check out this blog post to see ...

1

I had a crack at it, I tried to replace the for loops with higher-order functions, and use ES6 let and const for variable declarations. I feel like the efficiency of the functions could be improved, but this could be a good starting point to refactoring. Also, fixed some formatting to make it a bit easier to read. const AWS = require('aws-sdk'); const fs =...

1

A recommendation that I give with most of my reviews, and which is relevant here, is a perusal of PEP 8, the official style guide for Python. Many of my comments below will reference that document. import boto3, os PEP 8 has a few things to say about this (in the section entitled Imports). First, generally you shouldn't put multiple modules on the same ...

1

I'm not familiar with Amazon's API at all. I consulted https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html about the request format, I hope this is the applicable reference. This is not a thorough review, I was mostly looking for security risks. You should add set -e at the beginning of the script so that it will exit if a ...

1

Your code is already pretty good by common standards, IMO. If it is possible, you can try to adopt ES7's async/await. The code will become more linear, while still asynchronous. Watch out for browser compatibility though. export async function main(event) { const data = await dynamo .get({ TableName: 'client', Key: { id: event.name } }) ....

1

This can be written a bit simpler: index="" for var in "$@"; do if [[ "${index}" != "" ]]; then PMAP[$index]=${#PARAM[@]} PARAM[${#PARAM[@]}]=$var index="" continue fi if [[ "$var" = '--headers' ]]; then index=headers continue fi ARGS[${#ARGS[@]}]="$var" done Like this: index= for var;... 1 For simple text transformations, if simple parameter expansion can handle it, it's good to use it, rather calling full-blown processes like sed. For example: date=${dateTime%%T*} host=${url#*//} host=${host%%/*} service=${host%%-*} region=${host#*-} region=\${host%%.*} And so on. Instead of a wall of echo like in the second half of the script, I would ...

1

Usually, the code is a bit worse, indentation-wise: if res, ok := decodedOutput["resource"].([]map[string]interface{}); ok { for _, resource := range res { for key := range resource { switch key { case "aws_instance": if rk, ok := resource[key].([]map[string]interface{}); ok { for ...

Only top voted, non community-wiki answers of a minimum length are eligible