Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question - is there multiple topic support? #5

Open
3 tasks done
flusflas opened this issue Jan 20, 2021 · 12 comments
Open
3 tasks done

Question - is there multiple topic support? #5

flusflas opened this issue Jan 20, 2021 · 12 comments

Comments

@flusflas
Copy link

flusflas commented Jan 20, 2021

My actions before raising this issue

Expected Behaviour

This connector cannot deal with more than one MQTT topic, as stated in https://github.com/openfaas/mqtt-connector/tree/master/chart/mqtt-connector and confirmed by my own tests with the connector. This seems normal as only one MQTT subscription is created. I'm wondering why this connector is limited to one single topic per instance.

Current Behaviour

Only one topic is allowed. If more than one topic are used (e.g. my-topic,my-second-topic), no errors are thrown but only the first topic seems to be used. Only messages published to the first topic my-topic will be received by the connector.

Possible Solution

I've added very simple multi-topic support using multiple subscriptions, but maybe I'm missing how the connector-sdk handles/splits the topic internally.

Your Environment

  • FaaS-CLI version ( Full output from: faas-cli version ):
    commit: c12d57c39ac4cc6eef3c9bba2fb45113d882432f
    version: 0.12.14

  • Docker version docker version (e.g. Docker 17.0.05 ): Docker 19.03.12

  • Are you using Docker Swarm or Kubernetes (FaaS-netes)? No

  • Operating System and version (e.g. Linux, Windows, MacOS): Linux

@alexellis
Copy link
Member

alexellis commented Jan 28, 2021

Please do us a favour and use the issue template, this is required for everyone and there are no special cases.

https://raw.githubusercontent.com/openfaas/mqtt-connector/master/.github/ISSUE_TEMPLATE.md

Incidentally, it looks like multiple topics may already be supported - https://github.com/openfaas/mqtt-connector/blob/master/main.go#L70

You can see an example usage, if it's of interest to you here -> https://github.com/packet-labs/iot/tree/master/openfaas/mqtt-connector

Ping me here again when you've updated the description of your issue.

@alexellis
Copy link
Member

/set title: Question - is there multiple topic support?

@derek derek bot changed the title Multi-topic support Question - is there multiple topic support? Jan 28, 2021
@flusflas
Copy link
Author

Description updated. Based on my tests (and the connector documentation), only one topic is allowed as only one MQTT subscription is being created.

@max-regan
Copy link

I was also hoping for this functionality. I believe the code @alexellis referred to actually allows a function to have multiple "topic" annotations, not for the MQTT connector to subscribe to multiple MQTT topics.

I'm not currently in a position to test, however I believe that you may be able to have some success with using topic wildcards such as '#', which would subscribe to every topic.

Related source: https://github.com/eclipse/paho.mqtt.golang/blob/master/topic.go

Unfortunately I'm not in a position to test right now. Hopefully that helps!

@flusflas
Copy link
Author

flusflas commented Mar 2, 2021

I've seen that in

mqtt-connector/main.go

Lines 93 to 95 in d6f8f43

opts.SetDefaultPublishHandler(func(client MQTT.Client, msg MQTT.Message) {
choke <- [2]string{msg.Topic(), string(msg.Payload())}
})
a default publish handler is being used, so all messages in all topics are being received. As far as I can see, it makes both the topic list and the subscription in
if token := client.Subscribe(*topic, byte(*qos), nil); token.Wait() && token.Error() != nil {
completely useless.

Wouldn't a MessageHandler callback on the subscription line make more sense? This will allow to listen only to the requested topics. Of course, only one topic per subscription will be allowed. I guess is in that scenario where my question about multi-topic support made sense.

@aslanpour
Copy link

I think multi-topic is supported by mqtt-connector.
It just needs to be made a multi-topic handler by using '#' .

For instance, the connector can set a topic as '/function/#'.
Function 'test1' can be annotated by topic='/function/test1'.
Function 'test2' can be annotated by topic='/function/test2'.
Function 'test3' can be annotated by topic='/function/test3'.

Then by publishing a message on the topic 'function/test1', the connector will be notified and will only invoke function 'test1'.

I tested this and worked.

I would just understand the openfaas components that an MQTT message visits in this trip.
My understanding is that the MQTT message is delivered to MQTT broker --> mqtt-connector --> openfaas gateway --> function and then results are sent back to mqtt-connector that I do not understand why and how we can catch the result from mqtt-connector.
In case of async, the mqtt message is delivered to MQTT broker --> mqtt-connector --> nats --> queue-worker --> function -and results are sent back to mqtt-connector that I do not understand how we can set a callback here.
If someone is familiar openfaas design, please correct me.

@alexellis
Copy link
Member

@flusflas you didn't fill out the issue template and left off the "Steps to reproduce this issue" part

If you haven't tried this out, how do you know it doesn't work?

And before we get ahead of ourselves, are you talkigng about 1000 topcis, or 3?

If it's a low amount of topics, just install the connector N times per topic. That's a simple and easy solution.

@aslanpour please feel free to try out the scenario you listed above with a wildcard topic using #

@aslanpour
Copy link

@alexellis I just tried the scenario out and works for me. So, no need for a connector per topic. If you need a demonstration, I should be able to give more info in this thread.

@alexellis
Copy link
Member

That'd be great. I can then add it to the README after that for everyone to benefit from.

@flusflas
Copy link
Author

Of course you can use a topic with wildcards to address more than one topic, but it doesn't allow multiple topics if they don't share a common structure or you don't want to use wildcards. I think the problem was clear enough and that I provided detailed information on where the problem was.

Anyway, I chose to create a fork of this repository to add this and other features to the connectors, so feel free to close this issue if the current behavior is fine for you.

@alexellis
Copy link
Member

alexellis commented Dec 23, 2021

When you're asking someone to do work for you (out of good will, voluntarily) and they say: "this isn't clear enough", replying with "it is clear enough" means that we have a bit of a communication breakdown.

You're perfectly welcome to fork this repository for your own needs. That's why it's licensed as MIT, however you cannot change the copyright notices and need to credit the source.

If you're using openfaas for production or in a proprietary solution, then you should consider becoming a sponsor. That is, if you would like to see us continue to maintain the project and make it available for you.

https://github.com/sponsors/openfaas

@alexellis
Copy link
Member

/lock: off topic

@derek derek bot locked and limited conversation to collaborators Dec 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants