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

Remove topologymanager module #80

Merged
merged 6 commits into from
Mar 23, 2023
Merged

Remove topologymanager module #80

merged 6 commits into from
Mar 23, 2023

Conversation

sajith
Copy link
Member

@sajith sajith commented Mar 22, 2023

Issue is #79: this module is being moved to pce, in atlanticwave-sdx/pce#98.

@sajith sajith self-assigned this Mar 22, 2023
@sajith sajith linked an issue Mar 22, 2023 that may be closed by this pull request
@coveralls
Copy link

coveralls commented Mar 22, 2023

Coverage Status

Coverage: 66.486% (-18.9%) from 85.412% when pulling e6584d8 on 98.remove-topology into f030997 on main.

@sajith
Copy link
Member Author

sajith commented Mar 22, 2023

The drop in coverage is due to honesty :-)

Previously reported coverage was measuring coverage of tests (see https://coveralls.io/builds/55212972), which is meaningless. With some changes in setup.cfg in this branch, now we're measuring actual test coverage of the library.

@sajith sajith marked this pull request as ready for review March 22, 2023 23:22
@YufengXin
Copy link
Collaborator

where does it (topologymanger) go?-:) is it ready to use in pre? thanks

@sajith
Copy link
Member Author

sajith commented Mar 23, 2023

where does it (topologymanger) go?-:) is it ready to use in pre? thanks

It was moved to pce, in atlanticwave-sdx/pce#96, and I'm making some more changes in atlanticwave-sdx/pce#98 (mainly error handling and some refactoring).

So sdx-controller will use sdx.pce.topology instead of sdx.datamodel.topologymanager, and that change is being done in atlanticwave-sdx/sdx-controller#119. I think these changes are pretty much close to ready, modulo bugs and some more possible refactoring, of course. But we can remove this code now from pce, because sdx-controller's main branch is set to use datamodel tagged 1.0.0a, so it will continue to work.

The reason for this set of changes is stated in atlanticwave-sdx/pce#95. Basically, some classes were added to represent connection requests and solutions in pce (instead of plain old arrays and dicts), and topology-related classes can share that code easily if we move them to pce.

@YufengXin
Copy link
Collaborator

Thanks. I asked because I need to add some code to take care of the vlan reservation for every path. They're mostly done in my own branch. I will find the right place to add.

@sajith
Copy link
Member Author

sajith commented Mar 23, 2023

Thanks. Should have made this change earlier on. Sorry about the hold up!

@sajith sajith merged commit 7674c89 into main Mar 23, 2023
@sajith sajith deleted the 98.remove-topology branch March 23, 2023 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Remove topologymanager module from datamodel
3 participants