search

facebookarchive / augmented-traffic-control

Augmented Traffic Control: A tool to simulate network conditions
https://facebook.github.io/augmented-traffic-control
Other
4.33k stars 600 forks source link

drf loses track of data #10

Closed zealws closed 9 years ago

zealws commented 9 years ago

When data is posted to the API to start shaping a particular IP, the data is coherent in the view, but is lost somewhere between the view and serializer.

The POST data in the view reflects the data sent by the client, but the data that the serializer receives is filled with None objects rather than the defaulted dicts, and the rate information is completely lost.

Posted data:

{
  "down": {
    "rate": 10000,
    "loss": {
      "percentage": 0.0,
      "correlation": 0.0
    },
    "delay": {
      "delay": 0,
      "jitter": 0,
      "correlation": 0.0
    },
    "corruption": {
      "percentage": 0.0,
      "correlation": 0.0
    },
    "reorder": {
      "percentage": 0.0,
      "correlation": 0.0,
      "gap": 0
    },
    "iptables_options": []
  },
  "up": {
    "rate": 10000,
    "loss": {
      "percentage": 0.0,
      "correlation": 0.0
    },
    "delay": {
      "delay": 0,
      "jitter": 0,
      "correlation": 0.0
    },
    "corruption": {
      "percentage": 0.0,
      "correlation": 0.0
    },
    "reorder": {
      "percentage": 0.0,
      "correlation": 0.0,
      "gap": 0
    },
    "iptables_options": []
  }
}

Data seen by view:

{
  "down": {
    "rate": 10000,
    "loss": {
      "percentage": 0.0,
      "correlation": 0.0
    },
    "delay": {
      "delay": 0,
      "jitter": 0,
      "correlation": 0.0
    },
    "corruption": {
      "percentage": 0.0,
      "correlation": 0.0
    },
    "reorder": {
      "percentage": 0.0,
      "correlation": 0.0,
      "gap": 0
    },
    "iptables_options": []
  },
  "up": {
    "rate": 10000,
    "loss": {
      "percentage": 0.0,
      "correlation": 0.0
    },
    "delay": {
      "delay": 0,
      "jitter": 0,
      "correlation": 0.0
    },
    "corruption": {
      "percentage": 0.0,
      "correlation": 0.0
    },
    "reorder": {
      "percentage": 0.0,
      "correlation": 0.0,
      "gap": 0
    },
    "iptables_options": []
  }
}

Data seen by ThriftSerializer:

{u'down': OrderedDict([(u'rate', 0), (u'loss', None), (u'delay', None), (u'corruption', None), (u'reorder', None), (u'iptables_options', None)]), u'up': OrderedDict([(u'rate', 0), (u'loss', None), (u'delay', None), (u'corruption', None), (u'reorder', None), (u'iptables_options', None)])}
zealws commented 9 years ago

@chantra: Do you have any thoughts about why this is happening?

My e2e tests depend on being able to reliably post data to the API, so until this is fixed e2e is blocked.

chantra commented 9 years ago

I am not sure. I cant really reproduce this using master. Using the payload you mentioned and posting that to http://localhost:8080/api/v1/shape/ , atcd is shaping correctly:

Feb  6 23:52:51 trusty Request startShaping TrafficControl(device=TrafficControlledDevice(controllingIP='10.0.2.2', c
ontrolledIP='10.0.2.2'), timeout=86400, settings=TrafficControlSetting(down=Shaping(loss=Loss(percentage=0.0, correla
tion=0.0), delay=Delay(delay=0, jitter=0, correlation=0.0), rate=10000, iptables_options=[], corruption=Corruption(pe
rcentage=0.0, correlation=0.0), reorder=Reorder(percentage=0.0, correlation=0.0, gap=0)), up=Shaping(loss=Loss(percen
tage=0.0, correlation=0.0), delay=Delay(delay=0, jitter=0, correlation=0.0), rate=10000, iptables_options=[], corrupt
ion=Corruption(percentage=0.0, correlation=0.0), reorder=Reorder(percentage=0.0, correlation=0.0, gap=0))))
chantra commented 9 years ago 
{u'down': OrderedDict([(u'rate', 0), (u'loss', None), (u'delay', None), (u'corruption', None), (u'reorder', None), (u'iptables_options', None)]), u'up': OrderedDict([(u'rate', 0), (u'loss', None), (u'delay', None), (u'corruption', None), (u'reorder', None), (u'iptables_options', None)])}

Isn't this the defaults for the serializer and the it get populated as the payload is de-serialized?

zealws commented 9 years ago

It's possible I broke something while testing. I'll try to repruduce on master and get back to you.

Yes, those are the defaults, but in my posted data, I submitted more detailed information about each field than that. In addition, having None values causes ThriftSerializer to fail to serialize the object.

zealws commented 9 years ago

I'm still having this issue on master:

The changes I made were to add some debugging information to help disagnose an unrelated issue (#11), and didn't cause it.

The traceback below is a bit obfuscated, but when I looked into it it was because fields were being set to None rather than the values provided by the API.

The traceback I get:

Traceback:
File "/usr/local/atc/venv/local/lib/python2.7/site-packages/django/core/handlers/base.py" in get_response
  111.                     response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "/usr/local/atc/venv/local/lib/python2.7/site-packages/django/views/decorators/csrf.py" in wrapped_view
  57.         return view_func(*args, **kwargs)
File "/usr/local/atc/venv/local/lib/python2.7/site-packages/django/views/generic/base.py" in view
  69.             return self.dispatch(request, *args, **kwargs)
File "/usr/local/atc/venv/local/lib/python2.7/site-packages/rest_framework/views.py" in dispatch
  407.             response = self.handle_exception(exc)
File "/usr/local/atc/venv/local/lib/python2.7/site-packages/rest_framework/views.py" in dispatch
  404.             response = handler(request, *args, **kwargs)
File "/usr/local/src/atc/atc/django-atc-api/atc_api/views.py" in decorator
  39.         return method(cls, request, service, *args, **kwargs)
File "/usr/local/src/atc/atc/django-atc-api/atc_api/views.py" in post
  96.         setting = setting_serializer.save()
File "/usr/local/atc/venv/local/lib/python2.7/site-packages/rest_framework/serializers.py" in save
  164.             self.instance = self.create(validated_data)
File "/usr/local/src/atc/atc/django-atc-api/atc_api/serializers.py" in create
  62.                 args[arg_name] = serializer.create(attrs[f_name])
File "/usr/local/src/atc/atc/django-atc-api/atc_api/serializers.py" in create
  62.                 args[arg_name] = serializer.create(attrs[f_name])
File "/usr/local/src/atc/atc/django-atc-api/atc_api/serializers.py" in create
  57.             if f_name not in attrs:

Exception Type: TypeError at /api/v1/shape/
Exception Value: argument of type 'NoneType' is not iterable
chantra commented 9 years ago

curl -H "Content-Type: application/json" ...

should fix your issue....

We should try to find a saner way to fail though

chantra commented 9 years ago

This is due to how django-rest-framework works:

http://www.django-rest-framework.org/api-guide/parsers/

Note: When developing client applications always remember to make sure you're setting the Content-Type header when sending data in an HTTP request.

If you don't set the content type, most clients will default to using 'application/x-www-form-urlencoded', which may not be what you wanted.

As an example, if you are sending json encoded data using jQuery with the .ajax() method, you should make sure to include the contentType: 'application/json' setting.