search

PelionIoT / mbed-connector-api-python

python library for using connector.mbed.com
Apache License 2.0
2 stars 2 forks source link

Don't pass arrays to handlers #5

Open bridadan opened 8 years ago

bridadan commented 8 years ago

This is a mirror of the issue @janjongboom brought up on the Node library: https://github.com/ARMmbed/mbed-connector-node/issues/12

In Node it makes sense that each time an event handler is called, it only has one set of data to operate on. Ex. The registration handler only receives one endpoint, not all endpoints that have recently registered.

I would think the same thing should apply to Python, though I admit I lack knowledge on the Python paradigms here.

cc @janjongboom

janjongboom commented 8 years ago

+1

BlackstoneEngineering commented 8 years ago

I disagree. Currently I pass the entire notification payload to each handler function, that way the user can have a single function to handle multiple different notification types. With the python idiom of for x in y: it makes it really easy to handle lists of data.

janjongboom commented 8 years ago

Grouping notification payloads is something Connector does to prevent sending too many HTTP requests, we should not leak this abstraction into user-space where it makes no sense at all.

bridadan commented 8 years ago

You can do the same behavior of for x in y: in Javascript, but when you're observing a specific event, wouldn't it make sense for each event to apply to only one device? It's just less code that the user has to write, which is always nice :)

BlackstoneEngineering commented 8 years ago

Maybe, will have to give this some thought, marking it as a bug for the moment and will follow up in the near future on it.

BlackstoneEngineering commented 8 years ago

I have decided to leave it as is. I am erring on the side of flexibility. Should the user want they can add a intermediate layer that does what you have described, but if they dont want to then they dont have to.

I would like to implement this behaviour as part of a higher lever abstraction api in a future package.

bridadan commented 8 years ago

Just be aware that this breaks from my implementation in the Node library. I think if you document it clearly it shouldn't be an issue, but just so you're aware.

BlackstoneEngineering commented 8 years ago

Understood. Examples examples examples

janjongboom commented 8 years ago

I still don't agree. The reason it works like this in Connector is because of their HTTP implementation which is very heavy. If they would have done (TCP) Sockets they would have just pumped notifications one-by-one over the line. We're leaking their implementation details into our user abstraction now.