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

Add MQTT Gateway Support #1988

Open
wants to merge 28 commits into
base: master
Choose a base branch
from
Open

Add MQTT Gateway Support #1988

wants to merge 28 commits into from

Conversation

timvw74
Copy link

@timvw74 timvw74 commented Feb 15, 2022

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

@CLAassistant
Copy link

CLAassistant commented Feb 15, 2022

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ iancooper
✅ timvw74
❌ Tim van Wijk


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.

}
public class MQTTMessagingGatewayConsumerConfiguration : MQTTMessagingGatewayConfiguration
{
public Queue<Message> queue { get; set; }
Copy link
Member

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?

Copy link
Author

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.

Copy link
Member

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.

Copy link
Member

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.

Copy link
Member

@iancooper iancooper left a 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

@iancooper
Copy link
Member

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

@iancooper
Copy link
Member

@timvw74 I can merge if you would like. We can ignore the failing AWS tests for SNS/SQS

@iancooper iancooper marked this pull request as draft February 21, 2022 10:04
@iancooper
Copy link
Member

@timvw74 I converted to a draft whilst we sort out your build

@iancooper iancooper added 2 - In Progress v9.next allocate to a v9 future release labels May 24, 2022
@iancooper
Copy link
Member

Just a note that we tend to disable test parallelization for XUnit by putting tests into a collection.

@timvw74 timvw74 marked this pull request as ready for review March 6, 2023 09:07
@timvw74 timvw74 closed this by deleting the head repository Mar 29, 2023
@timvw74 timvw74 reopened this Mar 29, 2023
@iancooper iancooper added help wanted 0 - Backlog and removed 2 - In Progress v9.next allocate to a v9 future release labels Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants