feat(nms): generate swagger api for typescript #12757
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for opening a PR! 💯
Howto
More infoPlease take a moment to read through the Magma project's
If this is your first Magma PR, also consider reading
|
github-actions
bot
added
component: nms
Neudrino
force-pushed
the
nms/typescript-api-generation
branch
from
cc6052e
to
f1ba4d5
Compare
pull-request-size
bot
added
size/L
thmsschmitt
force-pushed
the
nms/typescript-api-generation
branch
3 times, most recently
from
ca72c95
to
c546c67
Compare
thmsschmitt
force-pushed
the
nms/typescript-api-generation
branch
from
c546c67
to
5099783
Compare
sebathomas
approved these changes
May 18, 2022
Comment on lines
61
to
63
| sed -i -e 's/?view=full(/_view_full(/g' "${file}" | ||
| sed -i -e 's/?verbose=false(/_verbose_false(/g' "${file}" |
There was a problem hiding this comment.
I assume this is the workaround for the hard-coded query parameters? Maybe add a TODO to remove them.
thmsschmitt
approved these changes
May 18, 2022
thmsschmitt
force-pushed
the
nms/typescript-api-generation
branch
from
5099783
to
7934353
Compare
Changing the name of the type is not breaking API, right? |
Correct, that is only an internal thing. |
maxhbr
reviewed
May 18, 2022
maxhbr
reviewed
May 18, 2022
orc8r/cloud/Makefile
Outdated
|
|
||
| nms_gen: | ||
| # generate Swagger API bindings for NMS | ||
| $(MAGMA_ROOT)/nms/scripts/generateAPIFromSwagger.sh $(SWAGGER_V1_YML) $(SWAGGER_NMS_OUT) | ||
| $(MAGMA_ROOT)/nms/scripts/generateAPIFromSwaggerForTypeScript.sh $(SWAGGER_V1_YML) $(SWAGGER_NMS_OUT_TS) |
There was a problem hiding this comment.
I think it would be more idiomatic to call a gen target in the nms makefile. But since it was done that way also previously, it might be fine.
thmsschmitt
reviewed
May 18, 2022
| echo "Input file: $INPUT"; | ||
| echo "Output directory: $OUTPUT"; | ||
|
|
||
| yarn --silent openapi -i "${INPUT}" -o "${OUTPUT}" |
There was a problem hiding this comment.
We have to set --client axios because we also used this API from node in a few places.
thmsschmitt
force-pushed
the
nms/typescript-api-generation
branch
from
7934353
to
b602119
Compare
maxhbr
reviewed
May 19, 2022
thmsschmitt
force-pushed
the
nms/typescript-api-generation
branch
from
b602119
to
ada007d
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
thmsschmitt
force-pushed
the
nms/typescript-api-generation
branch
from
ada007d
to
b49efa5
Compare
maxhbr
approved these changes
May 19, 2022
Signed-off-by: Fritz Lehnert <13189449+Neudrino@users.noreply.github.com>
Signed-off-by: Fritz Lehnert <13189449+Neudrino@users.noreply.github.com>
Signed-off-by: Fritz Lehnert <13189449+Neudrino@users.noreply.github.com>
…ackageType usage Signed-off-by: Thomas Schmitt <thomas.schmitt@tngtech.com>
Neudrino
force-pushed
the
nms/typescript-api-generation
branch
from
b49efa5
to
dce6a29
Compare
|
A follow up is at #12761. |
mpfirrmann
pushed a commit
to mpfirrmann/magma
that referenced
this pull request
May 31, 2022
* feat(nms): generate swagger api for typescript Signed-off-by: Fritz Lehnert <13189449+Neudrino@users.noreply.github.com> * feat(nms): rename package in swagger api Signed-off-by: Fritz Lehnert <13189449+Neudrino@users.noreply.github.com> * feat(nms): auto-generated files from swagger Signed-off-by: Fritz Lehnert <13189449+Neudrino@users.noreply.github.com> * refactor(nms): Make TypeScript generation script executable and fix PackageType usage Signed-off-by: Thomas Schmitt <thomas.schmitt@tngtech.com> Co-authored-by: Thomas Schmitt <thomas.schmitt@tngtech.com>
emakeev
pushed a commit
to emakeev/magma
that referenced
this pull request
Aug 5, 2022
* feat(nms): generate swagger api for typescript Signed-off-by: Fritz Lehnert <13189449+Neudrino@users.noreply.github.com> * feat(nms): rename package in swagger api Signed-off-by: Fritz Lehnert <13189449+Neudrino@users.noreply.github.com> * feat(nms): auto-generated files from swagger Signed-off-by: Fritz Lehnert <13189449+Neudrino@users.noreply.github.com> * refactor(nms): Make TypeScript generation script executable and fix PackageType usage Signed-off-by: Thomas Schmitt <thomas.schmitt@tngtech.com> Co-authored-by: Thomas Schmitt <thomas.schmitt@tngtech.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Generate the Swagger API for TypeScript in parallel to the one for Flow.
This is a preparatory step for migrating the NMS to TypeScript.
The tool the we are using for the flow API generation is discontinued and generates a broken TypeScript API.
Therefore we had to use a different tool for TypeScript.
We found an issues in our current swagger spec that we had to work around. There are three endpoints that return a differently shaped response based on a query parameter. This has been realised by putting the URL a second time into the swagger spec with a hard coded query parameter, which is not allowed. We should probably fix that by splitting the endpoints at some point.
Another problem that we ran into is more related to the generation tool. It will create types with reserved keywords. Since this only effected a very sparingly used subtype we renamed the type to unblock us.
The TypeScript API will be added in a separate PR #12761.
Fixes #12756
Done in pairing with @thmsschmitt .