Add REST API to ArduPilotManager #119
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
rafaellehmkuhl
requested review from
patrickelectric,
jaxxzer and
Williangalvani
patrickelectric
requested changes
Mar 20, 2021
rafaellehmkuhl
force-pushed
the
add-rest-api-ardupilot-manager
branch
2 times, most recently
from
1846a28
to
7b7d805
Compare
patrickelectric
requested changes
Mar 22, 2021
rafaellehmkuhl
force-pushed
the
add-rest-api-ardupilot-manager
branch
from
7b7d805
to
1801744
Compare
rafaellehmkuhl
force-pushed
the
add-rest-api-ardupilot-manager
branch
from
1801744
to
f0f444e
Compare
patrickelectric
requested changes
Mar 23, 2021
rafaellehmkuhl
force-pushed
the
add-rest-api-ardupilot-manager
branch
3 times, most recently
from
028ae13
to
237ece0
Compare
patrickelectric
requested changes
Mar 26, 2021
rafaellehmkuhl
force-pushed
the
add-rest-api-ardupilot-manager
branch
from
237ece0
to
576720f
Compare
There was a problem hiding this comment.
I did run python setup.py install and getting:
./main.py
Traceback (most recent call last):
File "/home/patrick/git/blue/companion-docker/core/services/ardupilot_manager/./main.py", line 7, in <module>
from fastapi import Body, FastAPI
File "/home/patrick/.local/lib/python3.9/site-packages/fastapi-0.63.0-py3.9.egg/fastapi/__init__.py", line 7, in <module>
from .applications import FastAPI as FastAPI
File "/home/patrick/.local/lib/python3.9/site-packages/fastapi-0.63.0-py3.9.egg/fastapi/applications.py", line 18, in <module>
from fastapi.openapi.utils import get_openapi
File "/home/patrick/.local/lib/python3.9/site-packages/fastapi-0.63.0-py3.9.egg/fastapi/openapi/utils.py", line 17, in <module>
from fastapi.responses import Response
File "/home/patrick/.local/lib/python3.9/site-packages/fastapi-0.63.0-py3.9.egg/fastapi/responses.py", line 10, in <module>
from starlette.responses import UJSONResponse as UJSONResponse # noqa
ImportError: cannot import name 'UJSONResponse' from 'starlette.responses' (/home/patrick/.local/lib/python3.9/site-packages/starlette/responses.py)
Could you run Edit: there was a change in starlette and they removed UJSONResponse in the newest version, but this should not occur here, since FastAPI froze starlette's version on 0.13.6 for backwards compatibility. |
|
There's probably some other package in your environment taking starlette to an upper version. Best match: starlette 0.13.6
Processing starlette-0.13.6-py3-none-any.whl
Installing starlette-0.13.6-py3-none-any.whl to /home/runner/.local/lib/python3.7/site-packages
Adding starlette 0.13.6 to easy-install.pth fileI suggest using a dedicated python virtual environment to isolate package's versions or downgrading your version of starlette to 0.13.6. |
Moved to a virtualenv solves the problem. |
patrickelectric
requested changes
Mar 26, 2021
rafaellehmkuhl
force-pushed
the
add-rest-api-ardupilot-manager
branch
from
576720f
to
1dfe415
Compare
This is exactly what IntEnum does, but for strings. In this way, json files and rest frameworks (like FastAPI) represent them as strings.
rafaellehmkuhl
force-pushed
the
add-rest-api-ardupilot-manager
branch
from
1dfe415
to
9869f75
Compare
patrickelectric
requested changes
Mar 26, 2021
rafaellehmkuhl
force-pushed
the
add-rest-api-ardupilot-manager
branch
from
9869f75
to
14d40b0
Compare
rafaellehmkuhl
requested review from
patrickelectric and
jaxxzer
and removed request for
jaxxzer
patrickelectric
approved these changes
Mar 29, 2021
Williangalvani
previously requested changes
Mar 29, 2021
rafaellehmkuhl
force-pushed
the
add-rest-api-ardupilot-manager
branch
from
14d40b0
to
b229b41
Compare
jaxxzer
reviewed
Mar 30, 2021
| if "/" in place: | ||
| raise ValueError(f"Bad filename: {place}. Valid filenames do not contain '/' characters.") | ||
| if argument is not None: | ||
| raise ValueError(f"File endpoint has should have no argument. Received {argument}.") |
There was a problem hiding this comment.
File endpoint has should have no argument.
jaxxzer
reviewed
Mar 30, 2021
| @@ -79,14 +79,11 @@ def start_navigator(self) -> None: | |||
| # does not accept TCP master endpoints | |||
| # self.start_mavlink_manager(Endpoint(local_endpoint)) | |||
|
|
|||
| # Check if subprocess is running and wait until it finishes | |||
| # Necessary since we don't have mavlink_manager running for navigator yet | |||
There was a problem hiding this comment.
minor detail, you don't need to fix it, but these comments should have come out in the previous commit :)
There was a problem hiding this comment.
Good catch!
Solved!
jaxxzer
approved these changes
Mar 30, 2021
- Make it a pydantic dataclass, for REST API validation auto-parsing - Adds validation to all arguments - Add dependency for endpoint validation - Add mypy plugin through configuration file
Initial version contains routes for getting, adding and deleting mavlink endpoints.
rafaellehmkuhl
force-pushed
the
add-rest-api-ardupilot-manager
branch
from
b229b41
to
ddc9446
Compare
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.
API is running as a uvicorn process, started with core.