ns1/nsone-python

NS1 Python SDK

nsone-python's Issues

Updating or adding an answer fails if answer is unicode.

Relevant lines here:

https://github.com/nsone/nsone-python/blob/master/nsone/rest/records.py#L34
https://github.com/nsone/nsone-python/blob/master/nsone/rest/records.py#L42

Line 34 above causes code to fail if answer is a unicode string.

Line 42 above causes code to fail if updating from an answer retrieved via loadRecord

Suggestion:

Use basestring instead of str. basestring is the common base class for both str and unicode types.

Python3.6 - Twisted HTTP client expects bytes

Hello,

While exploring the NS1 client, I was unable to list the zones for a certain API key. This appears to be an issue with either Twisted, or the way that NS1 calls into Twisted.

The code to reproduce, below, was tested with Twisted 17.1.0 and Twisted 17.5.0.

$ python3.6 -m venv virtualenv
$ pip install pyopenssl service_identity 'twisted[tls]==17.1.0' nsone
$ . virtualenv/bin/activate && python

>>> from nsone import Config, NSONE
>>> c = Config('./conf/nsone.json')
>>> a = NSONE(config=c)
>>> a.zones().list()

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/m/Documents/src/gitlab.com/tenome/mono-repo/interview/ctci-go/actual/ns1/virtualenv/lib/python3.6/site-packages/nsone/rest/zones.py", line 56, in list
    errback=errback)
  File "/Users/m/Documents/src/gitlab.com/tenome/mono-repo/interview/ctci-go/actual/ns1/virtualenv/lib/python3.6/site-packages/nsone/rest/resource.py", line 73, in _make_request
    return self._transport.send(type, self._make_url(path), **kwargs)
  File "/Users/m/Documents/src/gitlab.com/tenome/mono-repo/interview/ctci-go/actual/ns1/virtualenv/lib/python3.6/site-packages/nsone/rest/transport/twisted.py", line 169, in send
    d = self.agent.request(method, str(url), theaders, bProducer)
  File "/Users/m/Documents/src/gitlab.com/tenome/mono-repo/interview/ctci-go/actual/ns1/virtualenv/lib/python3.6/site-packages/twisted/web/client.py", line 1623, in request
    parsedURI = URI.fromBytes(uri)
  File "/Users/m/Documents/src/gitlab.com/tenome/mono-repo/interview/ctci-go/actual/ns1/virtualenv/lib/python3.6/site-packages/twisted/web/client.py", line 630, in fromBytes
    scheme, netloc, path, params, query, fragment = http.urlparse(uri)
  File "/Users/m/Documents/src/gitlab.com/tenome/mono-repo/interview/ctci-go/actual/ns1/virtualenv/lib/python3.6/site-packages/twisted/web/http.py", line 180, in urlparse
    raise TypeError("url must be bytes, not unicode")
TypeError: url must be bytes, not unicode

Here is the nsone.json configuration file:

{
  "api_version": "v1",
  "default_key": "key-one",
  "verbosity": 5,
  "transport": "twisted",
  "keys": {
    "key-one": {
      "key": "<<REDACTED>>",
      "desc": "NS1 API Key",
      "writeLock": true
    }
  },
  "cli": {
    "output_format": "text"
  }
}

Update fails silently

I was testing an implementation of the record update method and found that it failed silently sometimes. My code was slightly wrong:

record.update(answers=new_address)

instead of

record.update(answers=[new_address])

but no exception was raised. I would have expected some sort of "record does not exist" or "incorrect data type" error.

GeoDNS/GeoIP support

I would like to use OctoDNS who depends on this library to sync multiple providers with GeoDNS support. Without support in this package, it would be hard to get support in octodns/octodns#33.

records module is broken on Python 3

As Python 3.0 release notes says:

The builtin basestring abstract type was removed. Use str instead.

But records module uses basestring in two places [1][2].

That bug was introduced as a fix to #1 issue. You can use six package's string_type constant or if you don't want to introduce a dependency you may just copy necessary bits from six.py module.

0.9.14 is broken in python 2.7.3

After upgrading to 0.9.14, listing zones results in

    context = ssl.create_default_context()
AttributeError: 'module' object has no attribute 'create_default_context'

Downgrading to 0.9.13 solved the problem.

UnboundLocalError when exception other than HTTPError occurs

 File "/usr/local/lib/python2.7/dist-packages/nsone/zones.py", line 239, in qps
    return stats.qps(zone=self.zone, callback=callback, errback=errback)
  File "/usr/local/lib/python2.7/dist-packages/nsone/rest/stats.py", line 29, in qps
    errback=errback)
  File "/usr/local/lib/python2.7/dist-packages/nsone/rest/resource.py", line 73, in _make_request
    return self._transport.send(type, self._make_url(path), **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/nsone/rest/transport/basic.py", line 62, in send
    body = resp.read()
UnboundLocalError: local variable 'resp' referenced before assignment

In our cronjob that gathers stats from nsone, we occasionally get a traceback that includes the lines above. The relevant code at

nsone-python/nsone/rest/transport/basic.py

Lines 55 to 64 in a095e03

	# Handle error and responses the same so we can
	# always pass the body to the handleProblem function
	try:
	resp = opener.open(request)
	except HTTPError as e:
	resp = e
	finally:
	body = resp.read()
	if resp.code != 200:
	handleProblem(resp.code, resp, body)

is

try:
    resp = opener.open(request)
except HTTPError as e:
    resp = e
finally:
    body = resp.read()

The finally block references a veriable that will be undefined if open throws an exception other than HTTPError.

The usage method for the entire account fails on http request

My code

nsone=NSONE(configFile=configFile, keyID='globaldots')
usage=nsone.stats().usage(period="30d", aggregate="true", by_tier="false")

This is what goes out to the server:
GET /v1/('stats/usage',)?aggregate=True&period=30d&by_tier=True HTTP/1.1"
and of course it fails with 404

Twisted's transport 2.7 vs 3.X incompatible dict method

Within this line https://github.com/ns1/nsone-python/blob/master/nsone/rest/transport/twisted.py#L136, dict#iteritems() is valid in 2.7, but not in 3.X, thus breaking compatibility with 3.X due to dict#iteritems() being replaced with dict#items(). While 2.7 has a dict#items() it does not return an iterator like 3.X's does.

LIbrary can hang waiting on response from NS1

I have a cron job that pulls stats from NS1 every minute. Right now I don't have any locking preventing more than one copy of this cron from running. Instances of this cron have started piling up.

$ ps -ef | grep 'dns_graphite.py' | wc -l
183

If I check what they are doing, I find them stuck reading from a socket.

bpitts@prod-iman-www3:~$ sudo strace -p 859
Process 859 attached - interrupt to quit
read(3, ^C <unfinished ...>
Process 859 detached
bpitts@prod-iman-www3:~$ sudo ls -l /proc/859/fd
total 0
lr-x------ 1 root root 64 Mar 16 07:52 0 -> pipe:[741839769]
lrwx------ 1 root root 64 Mar 16 07:52 1 -> /tmp/tmpf6sjXC6 (deleted)
lrwx------ 1 root root 64 Mar 16 07:52 2 -> /tmp/tmpf6sjXC6 (deleted)
lrwx------ 1 root root 64 Mar 16 07:52 3 -> socket:[741845547]
lr-x------ 1 root root 64 Mar 16 07:52 8 -> pipe:[741839777]
l-wx------ 1 root root 64 Mar 16 07:52 9 -> pipe:[741839777]
bpitts@prod-iman-www3:~$ sudo lsof | grep 741845547
python      859       root    3u     IPv4          741845547       0t0        TCP prod-iman-www3.aws-us-east-1.evbops.com:31146->104.20.86.93:https (ESTABLISHED)

The other end of that socket is NS1.

coconut:~ bpitts$ host api.nsone.net
api.nsone.net has address 104.20.85.93
api.nsone.net has address 104.20.86.93

Regardless of the status of those servers, requests to them should not wait indefinitely. Currently they do because you do not set any timeouts: https://github.com/ns1/nsone-python/search?utf8=%E2%9C%93&q=timeout&type=Code

I think all your transports support timeouts, you just need to set them; e.g. http://docs.python-requests.org/en/latest/user/advanced/#timeouts