8000 MF-1087 - Remove WebSocket adapter by manuio · Pull Request #1120 · absmach/supermq · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

MF-1087 - Remove WebSocket adapter #1120

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

Merged
merged 6 commits into from
Apr 17, 2020
Merged

MF-1087 - Remove WebSocket adapter #1120

merged 6 commits into from
Apr 17, 2020

Conversation

manuio
Copy link
Contributor
@manuio manuio commented Apr 16, 2020

Signed-off-by: Manuel Imperiale manuel.imperiale@gmail.com

Resolves #1087

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
@manuio manuio requested a review from a team as a code owner April 16, 2020 18:04
drasko
drasko previously approved these changes Apr 16, 2020
Copy link
Contributor
@drasko drasko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@drasko
Copy link
Contributor
drasko commented Apr 16, 2020

I approved this one.

However - one thing to check: NginX conf, does it use WS only over MQTT?

Additionally, https://github.com/mainflux/mainflux/issues/1088 must be addressed quickly after this one is merged, as CT becomes redundant.

@manuio
Copy link
Contributor Author
manuio commented Apr 16, 2020

@drasko I removed from Nginx conf the ws-adapter endpoint. Correct me if I'm wrong but I think that MQTT over WS is configured here: https://github.com/mainflux/mainflux/blob/master/docker/nginx/nginx-key.conf#L89

I'll take #1088 when we are done with this one.

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
mteodor
mteodor previously approved these changes Apr 17, 2020
Comment on lines -141 to -152
if (r.uri.startsWith('/ws') && (!auth || !auth.length)) {
var a;
for (a in r.args) {
if (a == 'authorization' && r.args[a] === clientKey) {
return clientKey;
}
}

r.error('Authorization param does not match certificate');
return '';
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't remove this. We want to keep MQTT over WS.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok!

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
dborovcanin
dborovcanin previously approved these changes Apr 17, 2020
@nmarcetic
Copy link
Collaborator
nmarcetic commented Apr 17, 2020

@manuio Can you just make sure you update the vendor ? I don't see any removal regarding WS like Gorilla package. Besides this LGTM.

@codecov-io
Copy link

Codecov Report

Merging #1120 into master will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1120      +/-   ##
==========================================
- Coverage   76.23%   76.22%   -0.01%     
==========================================
  Files          96       94       -2     
  Lines        6721     6546     -175     
==========================================
- Hits         5124     4990     -134     
+ Misses       1251     1220      -31     
+ Partials      346      336      -10     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f982b8c...20b2996. Read the comment docs.

manuio added 2 commits April 17, 2020 11:53
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
@manuio manuio merged commit 01aa419 into absmach:master Apr 17, 2020
@manuio manuio deleted the ws branch April 17, 2020 12:05
dborovcanin pushed a commit that referenced this pull request Apr 17, 2020
* MF-1087 - Remove WebSocket adapter

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Rm all ws directory

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Revert authorization.js

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Remove /ws endpoint from ssl/authorization.js

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Rm gorilla from vendor

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Revert gorilla to vendor

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
dborovcanin pushed a commit that referenced this pull request Apr 17, 2020
* MF-1087 - Remove WebSocket adapter

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Rm all ws directory

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Revert authorization.js

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Remove /ws endpoint from ssl/authorization.js

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Rm gorilla from vendor

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Revert gorilla to vendor

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
dborovcanin pushed a commit that referenced this pull request Apr 17, 2020
* MF-1087 - Remove WebSocket adapter

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Rm all ws directory

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Revert authorization.js

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Remove /ws endpoint from ssl/authorization.js

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Rm gorilla from vendor

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Revert gorilla to vendor

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
manuio added a commit that referenced this pull request Oct 12, 2020
* MF-1087 - Remove WebSocket adapter

Signed-off-
8000
by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Rm all ws directory

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Revert authorization.js

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Remove /ws endpoint from ssl/authorization.js

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Rm gorilla from vendor

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Revert gorilla to vendor

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove WebSocket adapter
6 participants
0