-
Notifications
You must be signed in to change notification settings - Fork 31
Create endpoint for MCC ARO requests #645
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
base: main
Are you sure you want to change the base?
Conversation
Pull reviewers statsStats of the last 120 days for UWOrbital:
|
Syzygicality
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just some minor refactoring :)
| :param filters: List of request statuses to filter by. If empty, no filtering is applied | ||
| :return: ARO requests matching the criteria | ||
| """ | ||
| with get_db_session() as session: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use ARORequestWrapper found in wrappers.py to do database querying
python_test/conftest.py
Outdated
| "gs.backend.data.data_wrappers.mcc_wrappers.packet_telemetry_wrapper", | ||
| "gs.backend.data.data_wrappers.mcc_wrappers.packet_wrapper", | ||
| "gs.backend.data.data_wrappers.mcc_wrappers.telemetry_wrapper", | ||
| "gs.backend.api.v1.mcc.endpoints.aro_requests", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When refactoring to use the ARORequestWrapper, pls remove from this from conftest
python_test/test_aro_requests_api.py
Outdated
| @@ -0,0 +1,137 @@ | |||
| from datetime import datetime | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls adjust if necessary to accommodate the refactoring
|
also pls resolve failing pytest check |
| from fastapi import APIRouter, Query | ||
|
|
||
| from gs.backend.api.v1.mcc.models.responses import ARORequestsResponse | ||
| from gs.backend.data.data_wrappers.aro_wrapper.aro_request_wrapper import get_all_requests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the abstracted wrappers which eddie created
|
|
||
|
|
||
| @aro_requests_router.get("/", response_model=ARORequestsResponse) | ||
| async def get_aro_requests( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am using query parameters to pass in the parameters currently. Does that work or should I switch to a request model?
| :param filters: List of request statuses to filter by. If empty, no filtering is applied | ||
| :return: ARO requests matching the criteria | ||
| """ | ||
| requests = get_all_requests(count, offset, filters) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure to refer to eddies wrappers!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.



Purpose
Closes #349.
Add GET endpoint for ARO requests.
New Changes
Testing
Explain tests that you ran to verify code functionality.
Outstanding Changes
If there are non-critical changes (i.e. additional features) that can be made to this feature in the future, indicate them here.