brettlangdon / node-dogapi

Datadog API Node.JS Client
https://brettlangdon.github.io/node-dogapi/
105 stars 45 forks source link

Added IPv6 support. #52

Closed Townsheriff closed 7 years ago

Townsheriff commented 7 years ago

Automatically determines if using IPv4 or IPv6 network.

brettlangdon commented 7 years ago

@Townsheriff sorry for the delay here, long weekend AFK.

Do you no longer need this change?

Townsheriff commented 7 years ago

@brettlangdon I still need IPv6 support, but I found out that en0 interface is not present on all machines, therefore this PR is not good for general public. I added environment variable in my fork to solve this, but I don't know how to solve it to work automatically. Should I make PR with new changes (environment flag)?

brettlangdon commented 7 years ago

Yeah, I was going to suggest a different approach. I was thinking we can let people set the family via a config option in here: https://github.com/brettlangdon/node-dogapi/blob/8e3f949883ae74cec8bce1a317535c80d4d34ef5/lib/client.js#L14-L19

Could do something like this.use_ipv6, but maybe the better approach is this.http_options = {}

Then we can do:

var _ = require('lodash');
var http_options = _.assign({
    hostname: this.api_host,
    port: 443,
    method: method.toUpperCase(),
    path: path
}, this.http_options);

which will allow people to configure the family via:

var dogapi = require('dogapi');

dogapi.initialize({
    http_options: {
        // You can control this via an environment param if you choose
        family: '6'
    }
});

Thoughts?

Townsheriff commented 7 years ago

I think passing own http_options is more flexible. Wrapping these options will just create more complexity in future. Great idea. 👍 Will you add this functionality or should I?

brettlangdon commented 7 years ago

:+1:

Your call, I'm happy to do the work, can probably get something EOD today or tomorrow morning, or else more than happy to accept a PR.

Townsheriff commented 7 years ago

Ok, I will do it. Hopefully today or tomorrow.

brettlangdon commented 7 years ago

:+1: