Azure / iot-edge-v1

Azure IoT Edge
http://azure.github.io/iot-edge/
Other
525 stars 258 forks source link

Why source property must be "mapping" in IotHub module #394

Open villepalo opened 7 years ago

villepalo commented 7 years ago

Is there any good reason, why the "source" property must be set to "mapping". I mean, if I do not need IdentityMapper module I still have to set this property as "mapping". Shouldn't links array in configuration file take care of this kind of routing? I think setting deviceName and deviceKey properties should be enough. Or am I missing something here?

darobs commented 7 years ago

It's a leftover bit of code from when we didn't have routing. There's no need for it now.

...although, it's also a check in case you set up a route that sends messages to the iothub module you didn't actually want to sent to IoTHub. I like the requirement that a message be flagged for transmission to the IoTHub, but you could argue that a message which has device ID and device key is already flagged for transmission to the IoTHub.

We do have a PBI for this. The code & test of it is pretty easy, but we'd have to confirm that we are not breaking anyone by removing the check.

kiranpradeep commented 7 years ago

@darobs Does the identity mapper module needs the source property(mapping) now? A devdoc(1) comment mentions it was meant for a module to reject it's own messages.

darobs commented 7 years ago

@kiranpradeep The pull request is there, but we haven't merged it yet - we've been pretty heads down on the V2 Edge features. This is a problem we had before routes were introduced. With the routes in place, this would only be a problem if you routed iothub module messages directly back to the iothub module, which would be an odd thing to do. Since we need to do a new release soon. I was intending on accepting #395 - It makes sense to actually pull the pull requests.

kiranpradeep commented 7 years ago

@darobs The pull request doesn't touch identity mapper module. It only changes iothub module. For completion, shouldn't it change identity mapper too?

villepalo commented 7 years ago

@kiranpradeep Good point! I'm not sure though. Someone could use the source property for other purposes as well. It might be, that someone has created a module which depends on identitymapper-module and uses this property. I don't know, maybe @darobs knows better :)

BTW, shouldn't this question/thread be in review comments, not here?