Conversation
Signed-off-by: danguddemi <danguddemi@gmail.com>
Signed-off-by: danguddemi <danguddemi@gmail.com>
Signed-off-by: danguddemi <danguddemi@gmail.com>
Create LICENSE
| return obj.as_dict(include_pk=False).popitem()[1] | ||
| else: | ||
| return obj.as_dict(include_pk=obj.dict_carry_pk) | ||
| return obj.as_dict(include_pk=False).popitem()[1] if obj.dict_collapse else obj.as_dict(include_pk=obj.dict_carry_pk) No newline at end of file |
There was a problem hiding this comment.
This was way more readable as multiple lines
There was a problem hiding this comment.
insignificant change either way
| """ | ||
| get list of answerable questions | ||
| """ | ||
| page = Serverless.args.get('page', 1) |
There was a problem hiding this comment.
int should be the third arg for page and page_size
|
|
||
| def get_all_terms(page, page_size, order_by): | ||
| return [t.as_dict() for t in sort_and_paginate(Term.query, order_by, page, page_size).all()] | ||
| return (t.as_dict() for t in sort_and_paginate(Term.query, order_by, page, page_size).all()) |
There was a problem hiding this comment.
https://stackoverflow.com/questions/43808180/python-3-x-list-comprehension-vs-tuple-generator
Switch back to list comprehension
| except TypeError: | ||
| raise Abort(400, f"{type} is not a valid Type") | ||
| except ValueError: | ||
| return Abort(400, f"Invalid literal {key} for type {type}") |
There was a problem hiding this comment.
Query params should be returned as the default value if you fail to convert using type instead of returning a 400 error
| results_obj = body | ||
| elif isinstance(body, list) or isinstance(body, str): | ||
| results_obj = {"data": body} | ||
| elif isinstance(body, GeneratorType): |
There was a problem hiding this comment.
We should be returning a list because list comprehension is much faster anyway. I'm fine with leaving this condition here, but it really shouldn't be hit if we can help it.
| if facets: | ||
| query = apply_sql_facets(LookupQuestion, query, facets) | ||
| return [q.as_dict() for q in sort_and_paginate(query, order_by, page, page_size).all()] | ||
| return (q.as_dict() for q in sort_and_paginate(query, order_by, page, page_size).all()) |
There was a problem hiding this comment.
https://stackoverflow.com/questions/43808180/python-3-x-list-comprehension-vs-tuple-generator
Switch back to list comprehension
| """ | ||
| get list of answerable questions | ||
| """ | ||
| page = Serverless.args.get('page', 1) |
There was a problem hiding this comment.
page and page_size should have type as int in the Serverless.args.get(...)
|
|
||
| def get_all_categories(page, page_size, order_by): | ||
| return [c.as_dict() for c in sort_and_paginate(QuestionCategory.query, order_by, page, page_size).all()] | ||
| return (c.as_dict() for c in sort_and_paginate(QuestionCategory.query, order_by, page, page_size).all()) |
There was a problem hiding this comment.
https://stackoverflow.com/questions/43808180/python-3-x-list-comprehension-vs-tuple-generator
Switch back to list comprehension
| cls.logger.info(res) | ||
| return res | ||
| except Exception as e: | ||
| return responsify(f"An unknown error occurred {e}", 400) |
There was a problem hiding this comment.
Shouldn't this be a 500 error code?
There was a problem hiding this comment.
debatable
400 -> Client error
500 -> Server error
I figured it was more likely that the client passed in a bad argument than the server failing to do something
| cls.logger.info(f"Arguments {stringParams} | {pathParams}") | ||
|
|
||
| try: | ||
| body, *context = func(pathParams) if pathParams else func() |
There was a problem hiding this comment.
In Flask, it casted path parameters based on a converter i.e. <int:term_id> passed term_id into the function as an integer. I think right now the URL path parameters are being passed in as strings. Is there a way to define their types as ints or strings (if we wanted them as strings) in the serverless config?
If that's not possible, you could probably get away with just casting all the url params to ints (and if there's an error on the conversion, leave it as a string).
There was a problem hiding this comment.
it returns them as strings, it doesn't automatically cast. Casting everything is a bad design decision as well, you shouldn't be using errors as conditionals/control-flow. If you need an int, you should manually cast that specific param.
|
Are we killing this PR and branch? |
2 participants
No description provided.