openalto / alto

Standard Application-Layer Traffic Optimization (ALTO) Toolset.
MIT License
2 stars 6 forks source link

Add providing OpenAPI specification(yaml) for northbound API in code-first manner. #46

Closed itaru2622 closed 1 year ago

itaru2622 commented 1 year ago

Fix #45

OpenAPI specification is generated from code and can be got by curl -X GET http://alto-frontend:8000/openapi.yaml This feature is powered by https://www.django-rest-framework.org/api-guide/schemas/#generating-an-openapi-schema

itaru2622 commented 1 year ago

@fno2010 clould you review this PR, please.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02 :tada:

Comparison is base (b7e8a8b) 79.67% compared to head (37049f0) 79.69%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #46 +/- ## ========================================== + Coverage 79.67% 79.69% +0.02% ========================================== Files 29 29 Lines 1948 1950 +2 Branches 368 368 ========================================== + Hits 1552 1554 +2 Misses 272 272 Partials 124 124 ``` | [Impacted Files](https://codecov.io/gh/openalto/alto/pull/46?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openalto) | Coverage Δ | | |---|---|---| | [src/alto/server/northbound/alto/urls.py](https://codecov.io/gh/openalto/alto/pull/46?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openalto#diff-c3JjL2FsdG8vc2VydmVyL25vcnRoYm91bmQvYWx0by91cmxzLnB5) | `96.29% <100.00%> (+0.29%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openalto). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openalto)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

fno2010 commented 1 year ago

It is a good idea to leverage OpenAPI. But I don't see the openapi.yaml file in this commit.

I feel the openapi.yaml should be a part of configuration file (etc/alto.conf or a YANG data instance later), or generated by the configuration.

itaru2622 commented 1 year ago

@fno2010 thank you for reviewing and response.

I want to confirm what you meant.

I think, there is some ways to generate and provide openapi.yaml

for generation timing a. command execution in terminal and store it as static file. b. when it gets access to some REST endpoint

for providing

  1. just provide as static file but not via REST.
  2. provide via REST for the above A or B

I took choices as {B,2} in this PR at current. in this choice, there is no gap between running source codes and openapi.yaml.

Do you mean you want to choose A as generation and configure providing way (1or 2) by configiration? note that this choice causes a gap between source codes and openapi.yaml when developer missed executing A after modification.

moreover, I am wondering if A can provide correct openapi definition for running environment because REST paths can be changed by configuration (alto.conf) at starting time.

I can add a parameter in alto.conf to controlling turn on/off {B,2} feature.

fno2010 commented 1 year ago

@itaru2622 Many thanks for your clarification. I misunderstood this PR. I thought it was trying to use OpenAPI to generate northbound code. But clearly, I'm wrong. It provides OpenAPI spec based on the running northbound so that other tools (e.g., OAM) can read it. Then it makes sense to me.