-
Notifications
You must be signed in to change notification settings - Fork 179
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
feat(robot_server): add system/time GET & PUT endpoints #6403
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
6526c88
-added GET & PUT time endpoints, added tests for system, addressed so…
sanni-t 9b1cc58
add docstring & fix linter error
sanni-t f13c8d5
move api/system/time to robot-server. Tests will fail.
sanni-t a3ae977
added tavern tests, updated error response, suppressed str_to_dict er…
sanni-t fbdad1b
forgot to remove redundant test
sanni-t da033b9
better error message
sanni-t File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Empty file.
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
from datetime import datetime | ||
from pydantic import BaseModel | ||
|
||
from robot_server.service.json_api import ResponseModel, ResponseDataModel, \ | ||
RequestDataModel, RequestModel | ||
|
||
|
||
class SystemTimeAttributes(BaseModel): | ||
systemTime: datetime | ||
|
||
|
||
SystemTimeResponseDataModel = ResponseDataModel[SystemTimeAttributes] | ||
|
||
SystemTimeResponse = ResponseModel[SystemTimeResponseDataModel, dict] | ||
|
||
SystemTimeRequest = RequestModel[RequestDataModel[SystemTimeAttributes]] |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
import logging | ||
from datetime import datetime | ||
from fastapi import APIRouter | ||
from robot_server.system import time | ||
from typing import Dict | ||
from robot_server.service.system import models as time_models | ||
from robot_server.service.json_api import ResourceLink | ||
|
||
router = APIRouter() | ||
log = logging.getLogger(__name__) | ||
|
||
""" | ||
These routes allows the client to read & update robot system time | ||
""" | ||
|
||
|
||
def _create_response(dt: datetime) \ | ||
-> time_models.SystemTimeResponse: | ||
"""Create a SystemTimeResponse with system datetime""" | ||
return time_models.SystemTimeResponse( | ||
data=time_models.SystemTimeResponseDataModel.create( | ||
attributes=time_models.SystemTimeAttributes( | ||
systemTime=dt | ||
), | ||
resource_id="time" | ||
), | ||
links=_get_valid_time_links(router) | ||
) | ||
|
||
|
||
def _get_valid_time_links(api_router: APIRouter) -> Dict[str, ResourceLink]: | ||
""" Get valid links for time resource""" | ||
return { | ||
"GET": ResourceLink(href=api_router.url_path_for( | ||
get_time.__name__)), | ||
"PUT": ResourceLink(href=api_router.url_path_for( | ||
set_time.__name__)) | ||
} | ||
|
||
|
||
@router.get("/system/time", | ||
description="Fetch system time & date", | ||
summary="Get robot's time status, which includes- current UTC " | ||
"date & time, local timezone, whether robot time is synced" | ||
" with an NTP server &/or it has an active RTC.", | ||
response_model=time_models.SystemTimeResponse | ||
) | ||
async def get_time() -> time_models.SystemTimeResponse: | ||
res = await time.get_system_time() | ||
return _create_response(res) | ||
|
||
|
||
@router.put("/system/time", | ||
description="Update system time", | ||
summary="Set robot time", | ||
response_model=time_models.SystemTimeResponse) | ||
async def set_time(new_time: time_models.SystemTimeRequest) \ | ||
-> time_models.SystemTimeResponse: | ||
sys_time = await time.set_system_time(new_time.data.attributes.systemTime) | ||
return _create_response(sys_time) |
Empty file.
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
from robot_server.service.errors import RobotServerError, CommonErrorDef | ||
|
||
|
||
class SystemException(RobotServerError): | ||
"""Base of all system exceptions""" | ||
pass | ||
|
||
|
||
class SystemTimeAlreadySynchronized(SystemException): | ||
""" | ||
Cannot update system time because it is already being synchronized | ||
via NTP or local RTC. | ||
""" | ||
def __init__(self, msg: str): | ||
super().__init__(definition=CommonErrorDef.ACTION_FORBIDDEN, | ||
reason=msg) | ||
|
||
|
||
class SystemSetTimeException(SystemException): | ||
"""Server process Failure""" | ||
def __init__(self, msg: str): | ||
super().__init__(definition=CommonErrorDef.INTERNAL_SERVER_ERROR, | ||
error=msg) |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
import asyncio | ||
import logging | ||
from typing import Dict, Tuple, Union | ||
from datetime import datetime, timezone | ||
from opentrons.util.helpers import utc_now | ||
from robot_server.system import errors | ||
|
||
log = logging.getLogger(__name__) | ||
|
||
|
||
def _str_to_dict(res_str) -> Dict[str, Union[str, bool]]: | ||
res_lines = res_str.splitlines() | ||
res_dict = {} | ||
|
||
for line in res_lines: | ||
if line: | ||
try: | ||
prop, val = line.split('=') | ||
res_dict[prop] = val if val not in ['yes', 'no'] \ | ||
else val == 'yes' # Convert yes/no to boolean value | ||
except (ValueError, IndexError) as e: | ||
log.error(f'Error converting timedatectl status line {line}:' | ||
f' {e}') | ||
return res_dict | ||
|
||
|
||
async def _time_status(loop: asyncio.AbstractEventLoop = None | ||
) -> Dict[str, Union[str, bool]]: | ||
""" | ||
Get details of robot's date & time, with specifics of RTC (if present) | ||
& status of NTP synchronization. | ||
:return: Dictionary of status params. | ||
""" | ||
proc = await asyncio.create_subprocess_shell( | ||
'timedatectl show', | ||
stdout=asyncio.subprocess.PIPE, | ||
stderr=asyncio.subprocess.PIPE, | ||
loop=loop or asyncio.get_event_loop() | ||
) | ||
out, err = await proc.communicate() | ||
return _str_to_dict(out.decode()) | ||
|
||
|
||
async def _set_time(time: str, | ||
loop: asyncio.AbstractEventLoop = None) -> Tuple[str, str]: | ||
""" | ||
:return: tuple of output of date --set (usually the new date) | ||
& error, if any. | ||
""" | ||
proc = await asyncio.create_subprocess_shell( | ||
f'date --utc --set \"{time}\"', | ||
stdout=asyncio.subprocess.PIPE, | ||
stderr=asyncio.subprocess.PIPE, | ||
loop=loop or asyncio.get_event_loop() | ||
) | ||
out, err = await proc.communicate() | ||
return out.decode(), err.decode() | ||
|
||
|
||
async def get_system_time(loop: asyncio.AbstractEventLoop = None) -> datetime: | ||
""" | ||
:return: Just the system time as a UTC datetime object. | ||
""" | ||
return utc_now() | ||
|
||
|
||
async def set_system_time(new_time_dt: datetime, | ||
loop: asyncio.AbstractEventLoop = None | ||
) -> datetime: | ||
""" | ||
Set the system time unless system time is already being synchronized using | ||
an RTC or NTPsync. | ||
Raise error with message, if any. | ||
:return: current date read. | ||
""" | ||
status = await _time_status(loop) | ||
if status.get('LocalRTC') is True or status.get('NTPSynchronized') is True: | ||
# TODO: Update this to handle RTC sync correctly once we introduce RTC | ||
raise errors.SystemTimeAlreadySynchronized( | ||
'Cannot set system time; already synchronized with NTP or RTC') | ||
else: | ||
new_time_dt = new_time_dt.astimezone(tz=timezone.utc) | ||
new_time_str = new_time_dt.strftime("%Y-%m-%d %H:%M:%S") | ||
log.info(f'Setting time to {new_time_str} UTC') | ||
_, err = await _set_time(new_time_str) | ||
if err: | ||
raise errors.SystemSetTimeException(err) | ||
return utc_now() |
51 changes: 51 additions & 0 deletions
51
robot-server/tests/integration/system/test_system_time.tavern.yaml
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
--- | ||
test_name: GET Time | ||
marks: | ||
- usefixtures: | ||
- run_server | ||
stages: | ||
- name: System Time GET request returns time in correct format | ||
request: | ||
url: "{host:s}:{port:d}/system/time" | ||
method: GET | ||
response: | ||
status_code: 200 | ||
json: | ||
data: | ||
attributes: | ||
systemTime: !re_search "^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+\\+\\d{2}:\\d{2}$" | ||
id: 'time' | ||
type: 'SystemTimeAttributes' | ||
links: | ||
GET: | ||
href: "/system/time" | ||
meta: null | ||
PUT: | ||
href: "/system/time" | ||
meta: null | ||
meta: null | ||
|
||
--- | ||
test_name: PUT Time | ||
marks: | ||
- usefixtures: | ||
- run_server | ||
stages: | ||
- name: System Time PUT request without a time returns a missing field error | ||
request: | ||
url: "{host:s}:{port:d}/system/time" | ||
method: PUT | ||
json: | ||
data: | ||
id: "time" | ||
type: "SystemTimeAttributes" | ||
attributes: {} | ||
response: | ||
status_code: 422 | ||
json: | ||
errors: | ||
- status: "422" | ||
title: "value_error.missing" | ||
detail: "field required" | ||
source: | ||
pointer: "/body/new_time/data/attributes/systemTime" |
Empty file.
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
import pytest | ||
from unittest.mock import patch | ||
from datetime import datetime, timezone | ||
from robot_server.system import time, errors | ||
|
||
|
||
@pytest.fixture | ||
def mock_system_time(): | ||
return datetime(2020, 8, 14, 21, 44, 16, tzinfo=timezone.utc) | ||
|
||
|
||
@pytest.fixture | ||
def mock_set_system_time(mock_system_time): | ||
with patch.object(time, 'set_system_time') as p: | ||
yield p | ||
|
||
|
||
@pytest.fixture | ||
def response_links(): | ||
return { | ||
"GET": { | ||
"href": "/system/time", | ||
"meta": None | ||
}, | ||
"PUT": { | ||
"href": "/system/time", | ||
"meta": None | ||
} | ||
} | ||
|
||
|
||
def test_raise_system_synchronized_error(api_client, | ||
mock_system_time, | ||
mock_set_system_time): | ||
mock_set_system_time.side_effect = errors.SystemTimeAlreadySynchronized( | ||
'Cannot set system time; already synchronized with NTP or RTC') | ||
|
||
response = api_client.put("/system/time", json={ | ||
"data": { | ||
"id": "time", | ||
"type": "SystemTimeAttributes", | ||
"attributes": {"systemTime": mock_system_time.isoformat()} | ||
} | ||
}) | ||
assert response.json() == {'errors': [{ | ||
'detail': 'Cannot set system time; already synchronized with NTP ' | ||
'or RTC', | ||
'status': '403', | ||
'title': 'Action Forbidden'}]} | ||
assert response.status_code == 403 | ||
|
||
|
||
def test_raise_system_exception(api_client, | ||
mock_system_time, | ||
mock_set_system_time): | ||
mock_set_system_time.side_effect = errors.SystemSetTimeException( | ||
'Something went wrong') | ||
|
||
response = api_client.put("/system/time", json={ | ||
"data": { | ||
"id": "time", | ||
"type": "SystemTimeAttributes", | ||
"attributes": {"systemTime": mock_system_time.isoformat()} | ||
} | ||
}) | ||
assert response.json() == {'errors': [{ | ||
'detail': 'Something went wrong', | ||
'status': '500', | ||
'title': 'Internal Server Error'}]} | ||
assert response.status_code == 500 | ||
|
||
|
||
def test_set_system_time(api_client, mock_system_time, | ||
mock_set_system_time, response_links): | ||
async def mock_side_effect(*args, **kwargs): | ||
return mock_system_time | ||
|
||
mock_set_system_time.side_effect = mock_side_effect | ||
|
||
# Correct request | ||
response = api_client.put("/system/time", | ||
json={ | ||
'data': { | ||
'attributes': { | ||
'systemTime': | ||
mock_system_time.isoformat()}, | ||
'id': 'time', | ||
'type': 'SystemTimeAttributes'}, | ||
}) | ||
assert response.json() == { | ||
'data': { | ||
'attributes': {'systemTime': mock_system_time.isoformat()}, | ||
'id': 'time', | ||
'type': 'SystemTimeAttributes'}, | ||
'links': response_links, | ||
'meta': None | ||
} | ||
assert response.status_code == 200 |
Empty file.
Oops, something went wrong.
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.
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.
This warning is not quite accurate. We need to configure tests to recognize coverage of tavern tests. I will make a ticket now.
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.
That's a really cool feature to display it inline though!