-
Notifications
You must be signed in to change notification settings - Fork 411
Conversation
apistar/compat.py
Outdated
|
||
|
||
try: | ||
# Ideally we subclass `_TemproraryFileWrapper` to present a clear __repr__ |
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.
s/_TemproraryFileWrapper/_TemporaryFileWrapper
apistar/server/injector.py
Outdated
|
||
def identity(self, parameter: Parameter): | ||
""" | ||
Each component needs a unique identifier string that we use for lookups |
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.
perhaps mention that it is a unique, case-insensitive name?
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.
Thanks for the review! The name isn't actually case-insensitive, I'm just using the lowercase for stylistic reasons.
if sys.version_info < (3, 6): | ||
dict_type = collections.OrderedDict | ||
else: | ||
dict_type = dict |
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.
Not sure, that we can assume, that dict has ordered keys since 3.6
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.
dict is ordered in cpython 3.6 as an implementation detail, and guaranteed ordered in 3.7+
We don't strictly need ordering, it's only for stylistic purposes of eg. returning JSON outputs that are always in the same way as the fields in the codebase, so it's not a massive problem if there are a few edge cases of platforms that could get unordered outputs, but I'd be happy if there's any stricter checking that we can do, to also check if we're on cpython
or some other implementation.
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.
The order-preserving aspect of this new implementation is considered an implementation detail and should not be relied upon
https://docs.python.org/3/whatsnew/3.6.html#whatsnew36-compactdict
I could not find anything about guaranteed dict order since 3.7+.
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.
The boss said it : https://mail.python.org/pipermail/python-dev/2017-December/151283.html
In case of 404 requests, the on_request hook is not called but the on_error is called. The on_error has no idea if the on_request things were done or not. |
if hasattr(hook, 'on_request') | ||
] | ||
|
||
self.on_response_functions = [self.render_response] + [ |
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.
I think this list and the on_error
list should be reversed so that request and response hooks are always at the same "depth" in the request stack. I went into more details why here.
Lots of big changes here. I'll keep the synopsis to a minimum...
coreapi
andcoreschema
- Schema generation and client library is now part of this package.Here's the README
TODO:
query|path|body
Type
.Type
and annotation information on schema.Type
subclasses to be used in validations.uvicorn
for ASGI Support - See ASGI support uvicorn#63Later:
url
to Section.types
/fields
instead oftypes
/validators
.assert
Type
and handlingallOf
relationships in JSON schema.url
notbase_url
for decode option from HTTPTransport.errors
override on type system.