Skip to content
This repository has been archived by the owner on May 9, 2023. It is now read-only.

Support for our dear friend ALB #13

Merged
merged 3 commits into from
Oct 5, 2016
Merged

Support for our dear friend ALB #13

merged 3 commits into from
Oct 5, 2016

Conversation

aarthykc
Copy link
Contributor

@aarthykc aarthykc commented Oct 4, 2016

Allows the user to give and ALB or ELB link and gives percentage split of all the requests for an ELB/ALB

cc @emilymcafee, @yhahn

Copy link

@emilymcafee emilymcafee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to my 👀 , @aarthykc ! Left a few small comments. Any idea what Circle needs to be happy?

}

function preFlightCheck(elbname, callback) {
var q = queue(6);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We aren't expecting 6 things to happen at once, yeah? only 2?

q.defer(getELBName, elbname);
q.defer(getALBName, elbname);
q.awaitAll(function (err, data) {
if (err) console.log('err', err);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll want to return this error to the callback -- this would be a legitimate error (ie anything not filtered out by LoadBalancerNotFound)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh, left over from debugging ! fixing :D

q.defer(getALBName, elbname);
q.awaitAll(function (err, data) {
if (err) console.log('err', err);
data = data.filter(d => { return d; });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

=> 😊

var parameter = {Names: [elbname]};
elbv2.describeLoadBalancers(parameter, function (err, data) {
if (err) {
if (err.code === 'LoadBalancerNotFound') return callback(null, null);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[style] return callback(null, null) can also be expressed as return callback()


tape('preflight check elb', function (assert) {
preFlightCheck('elb', function (err, data) {
if (err) console.log(err);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be an assertion: assert.ifError(err)


tape('preflight check alb', function (assert) {
preFlightCheck('alb', function (err, data) {
if (err) console.log(err);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as ^^

@aarthykc
Copy link
Contributor Author

aarthykc commented Oct 5, 2016

@emilymcafee, fixed the tests + style issues ! Could you 👀 whenever you get a moment ?

@emilymcafee
Copy link

@aarthykc awesome ! Let's 🚢

@aarthykc aarthykc merged commit f40b555 into master Oct 5, 2016
@aarthykc aarthykc deleted the alb branch October 5, 2016 08:26
@yhahn yhahn mentioned this pull request Oct 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants