Skip to content
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

refactor(robot-server): add time link to /health, refactor system/time links #6517

Merged
merged 3 commits into from
Sep 11, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion robot-server/robot_server/service/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class CommonErrorDef(ErrorDef):
NOT_IMPLEMENTED = ErrorCreateDef(
status_code=status_codes.HTTP_501_NOT_IMPLEMENTED,
title='Not implemented',
format_string='Method not implemented')
format_string='Method not implemented. {error}')
RESOURCE_NOT_FOUND = ErrorCreateDef(
status_code=status_codes.HTTP_404_NOT_FOUND,
title='Resource Not Found',
Expand Down
6 changes: 5 additions & 1 deletion robot-server/robot_server/service/legacy/models/health.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ class Links(BaseModel):
apiSpec: str = \
Field(...,
description="The URI to this API specification")
systemTime: str = \
Field(...,
description="The URI for system time information")


class Health(BaseModel):
Expand Down Expand Up @@ -60,7 +63,8 @@ class Config:
"links": {
"apiLog": "/logs/api.log",
"serialLog": "/logs/serial.log",
"apiSpec": "/openapi.json"
"apiSpec": "/openapi.json",
"systemTime": "/system/time"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,6 @@ async def get_health(
links=Links(
apiLog='/logs/api.log',
serialLog='/logs/serial.log',
apiSpec="/openapi.json"
apiSpec="/openapi.json",
systemTime="/system/time"
))
14 changes: 1 addition & 13 deletions robot-server/robot_server/service/system/router.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@
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__)
Expand All @@ -24,20 +22,10 @@ def _create_response(dt: datetime) \
),
resource_id="time"
),
links=_get_valid_time_links(router)
links={'self': '/system/time'}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to our chat this morning and #6526, should this be a ResourceLink instead?

        links={'self': ResourceLink(href='/system/time'}}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha! Didn't see PR 6526

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From offline conversation, @amitlissack will be updating the time links to the new format as part of #6526

)


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 "
Expand Down
9 changes: 6 additions & 3 deletions robot-server/robot_server/system/errors.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from robot_server.service.errors import RobotServerError, CommonErrorDef
from robot_server.service.errors import RobotServerError, \
CommonErrorDef, ErrorDef


class SystemException(RobotServerError):
Expand All @@ -18,6 +19,8 @@ def __init__(self, msg: str):

class SystemSetTimeException(SystemException):
"""Server process Failure"""
def __init__(self, msg: str):
super().__init__(definition=CommonErrorDef.INTERNAL_SERVER_ERROR,
def __init__(self, msg: str, definition: ErrorDef = None):
if definition is None:
definition = CommonErrorDef.INTERNAL_SERVER_ERROR
super().__init__(definition=definition,
error=msg)
6 changes: 6 additions & 0 deletions robot-server/robot_server/system/time.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
from typing import Dict, Tuple, Union
from datetime import datetime, timezone
from opentrons.util.helpers import utc_now
from opentrons.config import IS_ROBOT
from robot_server.system import errors
from robot_server.service.errors import CommonErrorDef

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -47,6 +49,10 @@ async def _set_time(time: str,
:return: tuple of output of date --set (usually the new date)
& error, if any.
"""
if not IS_ROBOT:
raise errors.SystemSetTimeException(
msg="Not supported on dev server.",
definition=CommonErrorDef.NOT_IMPLEMENTED)
proc = await asyncio.create_subprocess_shell(
f'date --utc --set \"{time}\"',
stdout=asyncio.subprocess.PIPE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,7 @@ stages:
id: 'time'
type: 'SystemTimeAttributes'
links:
GET:
href: "/system/time"
meta: null
PUT:
href: "/system/time"
meta: null
self: '/system/time'
meta: null

---
Expand All @@ -49,3 +44,20 @@ stages:
detail: "field required"
source:
pointer: "/body/new_time/data/attributes/systemTime"
- name: System Time PUT request on a dev server raises error
request:
url: "{host:s}:{port:d}/system/time"
method: PUT
json:
data:
id: "time"
type: "SystemTimeAttributes"
attributes:
systemTime: "2020-09-10T21:00:15.741Z"
response:
status_code: 501
json:
errors:
- status: "501"
title: "Not implemented"
detail: "Method not implemented. Not supported on dev server."
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ stages:
serialLog: /logs/serial.log
# Specific link can change.
apiSpec: !re_match "/openapi.*"
systemTime: /system/time
3 changes: 2 additions & 1 deletion robot-server/tests/service/legacy/routers/test_health.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ def test_health(api_client, hardware):
"links": {
"apiLog": "/logs/api.log",
"serialLog": "/logs/serial.log",
"apiSpec": "/openapi.json"
"apiSpec": "/openapi.json",
"systemTime": "/system/time"
}
}
resp = api_client.get('/health')
Expand Down
11 changes: 1 addition & 10 deletions robot-server/tests/service/system/test_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,7 @@ def mock_set_system_time(mock_system_time):

@pytest.fixture
def response_links():
return {
"GET": {
"href": "/system/time",
"meta": None
},
"PUT": {
"href": "/system/time",
"meta": None
}
}
return {'self': '/system/time'}


def test_raise_system_synchronized_error(api_client,
Expand Down