Skip to content

Conversation

@Amuhar
Copy link

@Amuhar Amuhar commented Aug 28, 2017

Pull request

This is the pull request for "server-to-Server Stream Management Support for ejabberd" GSOC 2017 project.

During GSOC 2017 mod_stream_mgmt_s2s was implemented. To view documentation for server-to-server stream management module follow link:

https://github.com/Amuhar/docs_for_0198_s2s/blob/master/mod_stream_mgmt_s2s.md

Tests for server part of implementation are provided (module sms2s_tests).

@weiss
Copy link
Member

weiss commented Aug 29, 2017

Thanks.

We should look into at least the following issues (I didn't get to review the code until yesterday due to my vacation, so Anna couldn't address these things earlier):

  • The client part of the code currently sends the <enable/> request without checking for the stream feature. We should check s2s_out_authenticated_features (the PR contains code for this, but it's commented out). This means that stream management won't be negotiated for dialback-authenticated connections, though. (XEP-0198 was written with c2s connections in mind and doesn't mention this issue.)
  • If the queue exceeds the limit, the connection should be killed. Currently, only an error message is sent (to the remote server rather than the sender).
  • If the remote server specifies a location attribute, the code should honor this.
  • The module shouldn't access the Mnesia table s2s directly.
  • At least parts of the code could be merged with mod_stream_mgmt.
  • Test cases for the s2s_out (the stream management client) code should be added. This requires some changes to our current test infrastructure.

@Neustradamus
Copy link

Nice job in GSoC this year!

@cromain cromain added this to the ejabberd 19.x milestone Jan 10, 2019
@badlop badlop modified the milestones: ejabberd 21.xx, ejabberd 21.01 Jan 29, 2021
@badlop badlop removed this from the ejabberd 21.xx milestone Apr 8, 2021
@mremond
Copy link
Member

mremond commented Apr 15, 2021

@weiss Not sure if you still have plan to merge this patch.
Please, let us know :)

@Neustradamus
Copy link

Neustradamus commented Jan 7, 2024

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.

6 participants