-
Notifications
You must be signed in to change notification settings - Fork 259
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
Add MQTT Gateway Support #1988
base: master
Are you sure you want to change the base?
Add MQTT Gateway Support #1988
Conversation
Tim van Wijk seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
src/Paramore.Brighter.MessagingGateway.MQTT/MQTTMessageProducer.cs
Outdated
Show resolved
Hide resolved
} | ||
public class MQTTMessagingGatewayConsumerConfiguration : MQTTMessagingGatewayConfiguration | ||
{ | ||
public Queue<Message> queue { get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about passing the queue in on the config. It feels as though the interface is not clean as a result. Is this for testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was added to be able to pass in an external Queue. I have removed it as it can be added later if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's think about our alternatives here, as it's a "surprise" coming from other transports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a blocker to going forward though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some notes
- We try to document all public methods with a view to what Intellisense will give you; we don't document internal or private ones
Minor remarks though. Once we get this cleaned up, as you are the only person working on it, we can pretty much wave your PRs through if they are just for this transport, with feedback so you can iterate |
@timvw74 I can merge if you would like. We can ignore the failing AWS tests for SNS/SQS |
@timvw74 I converted to a draft whilst we sort out your build |
Just a note that we tend to disable test parallelization for XUnit by putting tests into a collection. |
Starter for MQTT Gateway support.
The Consumer Client keeps a local
Systems.Collections.Generic.Queue
, due to MQTT's "push" technology.Not implemented:
IAmAMessageConsumer.Acknowledge
IAmAMessageConsumer.Reject
IAmAMessageConsumer.Requeue