search

atlanticwave-sdx / sdx-controller

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

Review connection id data type and allowed values on swagger #257

Closed italovalcy closed 5 months ago

italovalcy commented 5 months ago

Hi,

Currently the connection ID is specified to be an Integer ranging from 1 to 10, which does not sounds reasonable. I think we should support creation of more than 10 "connections". Any reason to limit the number of connections?

https://github.com/atlanticwave-sdx/sdx-controller/blob/main/sdx_controller/swagger/swagger.yaml#L175-L176

Additionally maybe the connection ID should be generated by the controller, not provided by the user! Specially because the connection id is being used to persist data on database. I think we shouldn't rely on the user input to guarantee uniqueness of important fields like this.

What do you guys think?

sajith commented 5 months ago

@italovalcy I too don't think integers make a good connection IDs, because integers do not meet uniqueness properties we require. I also agree that the controller should generate connection IDs.

That is what I'm attempting in PR #225: POST /connection handler will generate a UUID to represent connection ID, and GET /connection/:connection_id and then DELETE /connection/:connection_id should use this generated ID. There are a couple of related issues mentioned there.

sajith commented 5 months ago

https://github.com/atlanticwave-sdx/sdx-controller/pull/225 made connection IDs integers, and removed the minimum and maximum limits. I believe that addresses this.