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

299 sdx controller must have new endpoints according to specification 20 #318

Conversation

YufengXin
Copy link
Collaborator

@YufengXin YufengXin commented Aug 19, 2024

spec v2:

  1. /connection -> /l2vpn
  2. adding the patch method

@YufengXin YufengXin linked an issue Aug 19, 2024 that may be closed by this pull request
@YufengXin YufengXin self-assigned this Aug 19, 2024
@YufengXin YufengXin marked this pull request as draft August 20, 2024 02:03
@coveralls
Copy link

coveralls commented Aug 20, 2024

Pull Request Test Coverage Report for Build 10464630503

Details

  • 251 of 355 (70.7%) changed or added relevant lines in 9 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+4.4%) to 57.931%

Changes Missing Coverage Covered Lines Changed/Added Lines %
sdx_controller/models/l2vpn_body.py 10 13 76.92%
sdx_controller/models/l2vpn_service_id_body.py 10 13 76.92%
sdx_controller/models/connection_v2_notifications.py 14 20 70.0%
sdx_controller/controllers/l2vpn_controller.py 9 18 50.0%
sdx_controller/models/connection_qos_unit.py 18 27 66.67%
sdx_controller/models/connection_scheduling.py 18 27 66.67%
sdx_controller/models/connection_v2_endpoints.py 18 27 66.67%
sdx_controller/models/connection_v2.py 107 133 80.45%
sdx_controller/models/connection_qos_metrics.py 47 77 61.04%
Totals Coverage Status
Change from base Build 10351089160: 4.4%
Covered Lines: 1038
Relevant Lines: 1858

💛 - Coveralls

@YufengXin YufengXin marked this pull request as ready for review August 20, 2024 03:16
@YufengXin
Copy link
Collaborator Author

YufengXin commented Aug 20, 2024

there are a few "unused" classes in models: for v2 and other future implementation

@YufengXin YufengXin merged commit 69c4617 into main Aug 20, 2024
11 checks passed
@YufengXin YufengXin deleted the 299-sdx-controller-must-have-new-endpoints-according-to-specification-20 branch August 20, 2024 14:30
@sajith
Copy link
Member

sajith commented Aug 20, 2024

there are a few "unused" classes in models: for v2 and other future implementation

It probably might be better to not use the generated models at all? A better way might be to write the models using pydantic in datamodel, and use them in the handlers and tests. That way we will be using Swagger Studio (or other tools) only to design the OpenAPI spec. I wrote up an issue about this: atlanticwave-sdx/datamodel#144.

From my quick testing, I think pydantic might work just fine for us. This might also solve atlanticwave-sdx/datamodel#92 in an elegant way.

@YufengXin
Copy link
Collaborator Author

Right! I'll learn and do...Thx

@sajith
Copy link
Member

sajith commented Aug 20, 2024

I am only weakly proposing this for now, to see if there is anything that I am missing because I have no experience with the current workflow (of generating code and updating it in sdx-controller and elsewhere). I will try writing some code with pydantic and see how well things work, and then propose changes more strongly if my assumptions about how things might work turn out to be true. :-)

We have higher priority things to do for now anyway. So this might be after October or so.

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.

SDX controller must have new endpoints according to specification 2.0
4 participants