atlanticwave-sdx / sdx-controller

Central Controller for AtlanticWave SDX.
https://www.atlanticwave-sdx.net
MIT License
1 stars 3 forks source link

Code cleaning on sdx_controller/messaging subfolder #249

Open italovalcy opened 3 months ago

italovalcy commented 3 months ago

As discussed on PR #223 (https://github.com/atlanticwave-sdx/sdx-controller/pull/223#issuecomment-1977043229) 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.

sajith commented 3 months ago

@italovalcy Thank you for raising this. I agree, that code should be refactored and simplified. I think these are the steps we should take:

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.

YufengXin commented 3 months ago

into the 'datamodel' repo? transformed to be the 'sdx-base'? just a thought.

sajith commented 3 months ago

Not sure. We should do the refactoring first, and then evaluate if we need a library at all.