You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
As discussed on PR #223 (#223 (comment)) there is opportunity to review the code under sdx_controller/messaging subfolder and remove unused code. I'm opening this issue here so we can discuss this and see if it makes sense or not to keep the code there.
@italovalcy Thank you for raising this. I agree, that code should be refactored and simplified. I think these are the steps we should take:
Removing unused code would be a good first step.
Reviewing, updating, documenting, and adding some tests for the remaining code would be the the next step.
There's some tight coupling of functionality, such as message handling and processing is done in a single method. Removing the coupling, and refactoring it into smaller, simpler methods would be useful.
We also create a thread within a thread (here). I'm not sure if we need it anymore (it was added before sdx-controller became a "proper" WSGI app). We should figure out the correct way.
Broadly, I think we just need a subscribe() method (which receives messages and pushes them to a queue for subsequent processing) and a publish() method (which sends messages to LCs). We could hash out the details during refactoring.
If done right, we can reuse the final subscribe() and publish methods in sdx-lc as well. I was thinking we could pull the code out into a library ("sdx-base" perhaps, along with other assorted utility methods that both sdx-controller and sdx-lc can share), but perhaps that might be overkill, if the amount of final code is pretty small.
As discussed on PR #223 (#223 (comment)) there is opportunity to review the code under
sdx_controller/messaging
subfolder and remove unused code. I'm opening this issue here so we can discuss this and see if it makes sense or not to keep the code there.Cc'ing @congwang09, @YufengXin and @sajith to provide comments.
The text was updated successfully, but these errors were encountered: