Skip to content

Make sure aiohttp web response body is a binary #199

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
10 changes: 5 additions & 5 deletions services/data/db_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,17 @@ def aiopg_exception_handling(exception):
body = {"err_msg": err_msg}
if isinstance(exception, psycopg2.IntegrityError):
if "duplicate key" in err_msg:
return DBResponse(response_code=409, body=json.dumps(body))
return DBResponse(response_code=409, body=body)
elif "foreign key" in err_msg:
return DBResponse(response_code=404, body=json.dumps(body))
return DBResponse(response_code=404, body=body)
else:
return DBResponse(response_code=500, body=json.dumps(body))
return DBResponse(response_code=500, body=body)
elif isinstance(exception, psycopg2.errors.UniqueViolation):
return DBResponse(response_code=409, body=json.dumps(body))
return DBResponse(response_code=409, body=body)
elif isinstance(exception, IndexError):
return DBResponse(response_code=404, body={})
else:
return DBResponse(response_code=500, body=json.dumps(body))
Copy link
Collaborator Author

@oavdeev oavdeev Jul 7, 2021

Choose a reason for hiding this comment

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

Unless I'm missing something, before this fix body was doubly-encoded here and at https://github.com/Netflix/metaflow-service/blob/master/services/metadata_service/api/utils.py#L24

Copy link
Contributor

Choose a reason for hiding this comment

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

This is somewhat duplicative of #184 (or the idea is for that to be the cleanup PR). I need to change things in the client too and we need to support both ways of doing things. But yes, this is planned.

return DBResponse(response_code=500, body=body)


def get_db_ts_epoch_str():
Expand Down
4 changes: 2 additions & 2 deletions services/metadata_service/api/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ async def get_authorization_token(self, request):
"SessionToken"
]

return web.Response(status=200, body=json.dumps(credentials))
return web.Response(status=200, body=json.dumps(credentials).encode('utf8'))
except Exception as ex:
body = {"err_msg": str(ex), "traceback": get_traceback_str()}
return web.Response(status=500, body=json.dumps(body))
return web.Response(status=500, body=json.dumps(body).encode('utf8'))
10 changes: 5 additions & 5 deletions services/metadata_service/api/artifact.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ async def get_artifacts_by_task(self, request):
filtered_body = filter_artifacts_by_attempt_id_for_tasks(
artifacts.body)
return web.Response(
status=artifacts.response_code, body=json.dumps(filtered_body)
status=artifacts.response_code, body=json.dumps(filtered_body).encode('utf8')
)

async def get_artifacts_by_step(self, request):
Expand Down Expand Up @@ -186,7 +186,7 @@ async def get_artifacts_by_step(self, request):
filtered_body = filter_artifacts_by_attempt_id_for_tasks(
artifacts.body)
return web.Response(
status=artifacts.response_code, body=json.dumps(filtered_body)
status=artifacts.response_code, body=json.dumps(filtered_body).encode('utf8')
)

async def get_artifacts_by_run(self, request):
Expand Down Expand Up @@ -221,7 +221,7 @@ async def get_artifacts_by_run(self, request):
filtered_body = filter_artifacts_by_attempt_id_for_tasks(
artifacts.body)
return web.Response(
status=artifacts.response_code, body=json.dumps(filtered_body)
status=artifacts.response_code, body=json.dumps(filtered_body).encode('utf8')
)

async def create_artifacts(self, request):
Expand Down Expand Up @@ -315,7 +315,7 @@ async def create_artifacts(self, request):
step_name, task_id)
except Exception:
return web.Response(status=400, body=json.dumps(
{"message": "need to register run_id and task_id first"}))
{"message": "need to register run_id and task_id first"}).encode('utf8'))

# todo change to bulk insert
for artifact in body:
Expand Down Expand Up @@ -343,4 +343,4 @@ async def create_artifacts(self, request):

result = {"artifacts_created": count}

return web.Response(body=json.dumps(result))
return web.Response(body=json.dumps(result).encode('utf8'))
4 changes: 2 additions & 2 deletions services/metadata_service/api/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ async def create_metadata(self, request):
step_name, task_id)
except Exception:
return web.Response(status=400, body=json.dumps(
{"message": "need to register run_id and task_id first"}))
{"message": "need to register run_id and task_id first"}).encode('utf8'))

for datum in body:
values = {
Expand All @@ -202,4 +202,4 @@ async def create_metadata(self, request):

result = {"metadata_created": count}

return web.Response(body=json.dumps(result))
return web.Response(body=json.dumps(result).encode('utf8'))
2 changes: 1 addition & 1 deletion services/metadata_service/api/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ async def create_task(self, request):

if task_name and task_name.isnumeric():
return web.Response(status=400, body=json.dumps(
{"message": "provided task_name may not be a numeric"}))
{"message": "provided task_name may not be a numeric"}).encode('utf8'))

run_number, run_id = await self._db.get_run_ids(flow_id, run_number)

Expand Down
4 changes: 2 additions & 2 deletions services/metadata_service/api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def format_response(func):
async def wrapper(*args, **kwargs):
db_response = await func(*args, **kwargs)
return web.Response(status=db_response.response_code,
body=json.dumps(db_response.body),
body=json.dumps(db_response.body).encode('utf8'),
headers=MultiDict(
{METADATA_SERVICE_HEADER: METADATA_SERVICE_VERSION}))

Expand All @@ -30,7 +30,7 @@ async def wrapper(*args, **kwargs):

def web_response(status: int, body):
return web.Response(status=status,
body=json.dumps(body),
body=json.dumps(body).encode('utf8'),
headers=MultiDict(
{"Content-Type": "application/json",
METADATA_SERVICE_HEADER: METADATA_SERVICE_VERSION}))
Expand Down
4 changes: 2 additions & 2 deletions services/migration_service/api/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,12 @@ async def db_schema_status(self, request):
"db_schema_versions": ApiUtils.list_migrations(),
"unapplied_migrations": unapplied_migrations
}
return web.Response(body=json.dumps(body),
return web.Response(body=json.dumps(body).encode('utf8'),
headers=MultiDict({"Content-Type": "application/json"}))

except Exception as e:
body = {
"detail": repr(e)
}
return web.Response(status=500, body=json.dumps(body),
return web.Response(status=500, body=json.dumps(body).encode('utf8'),
headers=MultiDict({"Content-Type": "application/json"}))