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 support #6

Closed
wants to merge 1 commit into from
Closed

Add MQTT support #6

wants to merge 1 commit into from

Conversation

filinvadim
Copy link

MQTT pubsub driver for Go Cloud Development Kit

REFERENCES:
google/go-cloud#1466

google/go-cloud#2751

@Kale-Grabovski
Copy link

How long does it take to review this pr?

@kpacha
Copy link
Member

kpacha commented Feb 29, 2020

I can't give an accurate estimate because we're closing a release, but we check and appreciate every issue and pr, so don't panic it will be review in a week or so.

@kpacha
Copy link
Member

kpacha commented Mar 6, 2020

@filinvadim, great job! I think the pkg is promising even at its early stage (the project is not very mature). After a quick review of the code there are some things I think could be problematic, such as using infinite waits at the subscribe and publish methods or the lack of parameters

please, let us know when the project is a little more mature and we'll resume the integration process

cheers!

@kpacha kpacha added the enhancement New feature or request label Mar 6, 2020
@filinvadim
Copy link
Author

filinvadim commented Mar 6, 2020

@kpacha this project and the whole code was copied from existing go-cloud pubsub drivers. So according your logic go-cloud immature too :)

@filinvadim
Copy link
Author

Infinite waits and lack of parameters were fixed.

@filinvadim
Copy link
Author

filinvadim commented Mar 10, 2020

@kpacha sorry for bothering you but my employer keeps poking me about this pr

@alombarte
Copy link
Member

@filinvadim we offer professional services and consultancy for Enterprises. This repository is open-source. Get in touch: support AT devops DOT faith.

@filinvadim
Copy link
Author

Guys?

@alombarte
Copy link
Member

Hello @filinvadim

Thanks for the time you spent on this new functionality. Although we don't have a market for this kind of feature currently, I was initially excited to see MQTT support, and I asked the team to go through this PR.

We take very seriously all the components and functionalities that are added in the gateway as they impact a broad base of users. Some of the things we evaluate internally are performance, long-term support, licensing, production usage of included libraries, etc.

The krakend pubsub component is included in all our KrakenD Enterprise offerings. It's a cornerstone for service connectivity and needs much more attention from our side than other pieces of the gateway as the impact of adding code is more prominent.
The first thing that refrains me for accepting the PR is that the library our project will depend on, cannot be considered to be yet in production. It seems that there is no usage or references to the library, only 1 contributor, and not backed (hosted) by a company:
https://pkg.go.dev/github.com/filinvadim/go-cloud-pubsub-mqtt?tab=importedby

The library has only 5 commits.

When going through the implementation, we've found other problems related to performance and configuration. I do not put in doubt the value of the contribution, and I am sure this solution will work for you, but I am not confident of adding it to all our user base.

My recommendation would be referencing your own component in your main.go (and will be your first imported of the library 😉). For now we won't merge the feature. We can review it again when there is official support of Google Cloud to the MQTT functionality.

Спасибо

@alombarte alombarte closed this Mar 18, 2020
@github-actions
Copy link

github-actions bot commented Apr 7, 2022

This pull request was marked as resolved a long time ago and now has been automatically locked as there has not been any recent activity after it. You can still open a new issue and reference this link.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants