Add ways to filter (or hook) request/response objects #47
Conversation
| DEFAULT_ERROR_CODE = 500 | ||
|
|
||
| def __init__(self, code=DEFAULT_ERROR_CODE, | ||
| description='something goes wrong.', url=None): |
There was a problem hiding this comment.
세 인자 모두 필수라서 굳이 디폴트가 필요 없을 것 같습니다.
| result = service_method(**arguments) | ||
| try: | ||
| result = service_method(**arguments) | ||
| except HttpError as e: |
There was a problem hiding this comment.
서비스 클라이언트 또는 서버를 구현하는 애플리케이션 코드에서 트랜스포트 계층(여기서는 HTTP)와 관계가 깊은 예외 타입을 직접 다루는 것이 적절치 않은 것 같습니다. 예를 들어 트랜스포트 계층이 HTTP였다가 AMQP로 바뀌거나 한다면 HTTP와 관련된 예외 코드들은 다 수정해야 하는데, 트랜스포트 계층과 애플리케이션 계층을 나누려는 원칙과는 상충됩니다.
| ) | ||
| ) | ||
|
|
||
| def _json_response(self, status_code, response_json, **kwargs): | ||
| def make_response(self, status_code, response_json, **kwargs): |
There was a problem hiding this comment.
_json_response()라는 이름은 그대로 두거나 _raw_response() 정도로 바꾸고, make_response()라는 빈 메서드를 새로 추가하는 것이 어떨까요? 이런 식이 되겠습니다.
def make_response(self, status_code, headers, content):
return status_code, headers, content
def _raw_response(self, status_code, response_json, **kwargs):
response_tuple = self.make_response(
status_code=status_code,
headers={},
content=json.dumps(response_json).encode('utf-8')
)
if not (isinstance(response_tuple, collections.Sequence) and
len(response_tuple) != 3):
raise TypeError('make_response() must return a triple of (status_code, '
'headers, content)')
status_code, headers, content = response_tuple
# 비슷하게 status_code, headers, content 각각에 대한 자료형이나 값을 검사.
# make_response()는 사용자가 오버라이드해서 쓰는 부분이므로 잘못됐을 때에 보는 오류 메세지가 구체적이야 하기 때문.
return WsgiResponse(content, status_code, headers, **kwargs)| 'Accepts': 'application/json'} | ||
| ) | ||
| return self.make_request(req) | ||
| return req |
There was a problem hiding this comment.
Server 쪽에서도 사용자가 정의할 수 있는 make_response()와 사용자가 동작을 바꿀 수 없는 _raw_response() 부분을 구분한 것처럼, 여기서도 make_request()의 이름을 바꿔서 구현 디테일 메서드로 나누고, make_request()라는 이름은 사용자가 오버라이드하는 용도로 남겨두는 게 좋을 것 같습니다.
| content_type='application/json', | ||
| **kwargs | ||
| def make_response(self, status_code, headers, content): | ||
| return status_code, content, headers |
There was a problem hiding this comment.
인자로 받을 때는 headers 먼저 받고 content를 마지막에 받는데, 반환할 때는 content가 먼저 오고 headers가 마지막에 오네요. 이거 헷갈리지 않을까요?
| 'make_response() must return a triple of ' | ||
| '(status_code, content, headers): {}'.format(response_tuple) | ||
| ) | ||
| status_code, content, headers = response_tuple |
There was a problem hiding this comment.
그리고, status_code, content, headers 각각에 대한 검사까지 위쪽에서 한번에 하고, 여러가지 오류의 경우 중에 하나만 해당해도 오류 메세지는 항상 똑같은데, status_code, content, headers 각각에 대한 검사는 tuple unpacking이 일어난 뒤에 따로 하는 게 읽기에도 좋고, 오류 메세지를 나누기에도 좋을 것 같습니다.
| len(response_tuple) == 3 and \ | ||
| isinstance(response_tuple[0], int) and \ | ||
| isinstance(response_tuple[1], str) and \ | ||
| isinstance(response_tuple[2], dict): |
There was a problem hiding this comment.
꼭 dict일 필요 없이 collections.Mapping 형식은 다 받아주는 게 좋을지도… 근데 생각해보니 HTTP 헤더는 같은 헤더가 중복될 수도 있어서 보통 Sequence[Tuple[str, str]] 형식으로 쓸 때가 많은 것 같습니다.
| ) | ||
| if not isinstance(response_tuple, collections.Sequence) and \ | ||
| len(response_tuple) == 3 and \ | ||
| isinstance(response_tuple[0], int) and \ |
There was a problem hiding this comment.
파이썬 2에서는 isinstance(200L, int)가 False일 겁니다.
| if not isinstance(response_tuple, collections.Sequence) and \ | ||
| len(response_tuple) == 3 and \ | ||
| isinstance(response_tuple[0], int) and \ | ||
| isinstance(response_tuple[1], str) and \ |
There was a problem hiding this comment.
HTTP 응답 내용은 str이 아니라 bytes여야 할 것 같습니다.
| 'Accepts': 'application/json'} | ||
| return self.do_request(request_url, payload) | ||
|
|
||
| def make_request(self, request_url, payload, headers): |
| if not isinstance(request_tuple, collections.Sequence) and \ | ||
| len(request_tuple) == 3 and \ | ||
| isinstance(request_tuple[0], str) and \ | ||
| isinstance(request_tuple[1], str) and \ |
There was a problem hiding this comment.
이쪽도 Server._raw_response()와 마찬가지입니다. HTTP 요청 내용은 유니코드 문자열 외에도 바이너리 데이터를 포함할 수 있으니 str 대신 bytes가 와야합니다. 파이썬 2와 파이썬 3에서 str의 의미가 다르기도 하고요.
| len(request_tuple) == 3 and \ | ||
| isinstance(request_tuple[0], str) and \ | ||
| isinstance(request_tuple[1], str) and \ | ||
| isinstance(request_tuple[2], dict): |
There was a problem hiding this comment.
이쪽도 Server._raw_response()와 마찬가지인데, HTTP 요청 헤더는 같은 헤더가 반복될 수 있어서 딕셔너리보다는 Sequence[Tuple[str, str]] 형태가 나은 듯합니다. 아니면 Union[Sequence[Tuple[str, str]], Mapping[str, str]] 형식도 괜찮을 것 같네요.
| 'make_request() must return a triple of ' | ||
| '(request_url, content, header): {}'.format(request_tuple) | ||
| ) | ||
| request_url, content, headers = request_tuple |
There was a problem hiding this comment.
이쪽도 Server._raw_response()와 마찬가지로, tuple unpacking 이후에 request_url, content, headers 각각에 대한 오류 처리를 해주는 게 좋을 것 같습니다.
#46