-
Notifications
You must be signed in to change notification settings - Fork 3
Sourcery refactored master branch #1
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,16 +14,11 @@ def is_authenticated(self): | |
| return True | ||
|
|
||
| def make_user_key(self, username): | ||
| return 'user_{}'.format(username) | ||
| return f'user_{username}' | ||
|
|
||
| def list(self): | ||
| usernames = self.conn.lrange('users', 0, 100) | ||
| users = [] | ||
|
|
||
| for user in usernames: | ||
| users.append(self.conn.hgetall(self.make_user_key(user))) | ||
|
|
||
| return users | ||
| return [self.conn.hgetall(self.make_user_key(user)) for user in usernames] | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
|
||
| def detail(self, username): | ||
| return self.conn.hgetall(self.make_user_key(username)) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,16 +14,11 @@ def is_authenticated(self): | |
| return True | ||
|
|
||
| def make_user_key(self, username): | ||
| return 'user_{}'.format(username) | ||
| return f'user_{username}' | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
|
||
| def list(self): | ||
| usernames = self.conn.lrange('users', 0, 100) | ||
| users = [] | ||
|
|
||
| for user in usernames: | ||
| users.append(self.conn.hgetall(self.make_user_key(user))) | ||
|
|
||
| return users | ||
| return [self.conn.hgetall(self.make_user_key(user)) for user in usernames] | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
|
||
| def detail(self, username): | ||
| return self.conn.hgetall(self.make_user_key(username)) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -60,24 +60,19 @@ def wrap_list_response(self, data): | |||||
|
|
||||||
| # Because Django. | ||||||
| @classmethod | ||||||
| def as_list(self, *args, **kwargs): | ||||||
| return csrf_exempt(super(DjangoResource, self).as_list(*args, **kwargs)) | ||||||
| def as_list(cls, *args, **kwargs): | ||||||
| return csrf_exempt(super(DjangoResource, cls).as_list(*args, **kwargs)) | ||||||
|
Comment on lines
-63
to
+64
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
It's unnecessary to use arguments when calling super for the parent class. Read more. |
||||||
|
|
||||||
| @classmethod | ||||||
| def as_detail(self, *args, **kwargs): | ||||||
| return csrf_exempt(super(DjangoResource, self).as_detail(*args, **kwargs)) | ||||||
| def as_detail(cls, *args, **kwargs): | ||||||
| return csrf_exempt(super(DjangoResource, cls).as_detail(*args, **kwargs)) | ||||||
|
Comment on lines
-67
to
+68
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
As above, These super arguments are unnecessary. |
||||||
|
|
||||||
| def is_debug(self): | ||||||
| return settings.DEBUG | ||||||
|
|
||||||
| def build_response(self, data, status=OK): | ||||||
| if status == NO_CONTENT: | ||||||
| # Avoid crashing the client when it tries to parse nonexisting JSON. | ||||||
| content_type = 'text/plain' | ||||||
| else: | ||||||
| content_type = 'application/json' | ||||||
| resp = HttpResponse(data, content_type=content_type, status=status) | ||||||
| return resp | ||||||
| content_type = 'text/plain' if status == NO_CONTENT else 'application/json' | ||||||
| return HttpResponse(data, content_type=content_type, status=status) | ||||||
|
Comment on lines
-74
to
+75
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
This removes the following comments ( why? ): |
||||||
|
|
||||||
| def build_error(self, err): | ||||||
| # A bit nicer behavior surrounding things that don't exist. | ||||||
|
|
@@ -105,9 +100,7 @@ def build_url_name(cls, name, name_prefix=None): | |||||
| :rtype: string | ||||||
| """ | ||||||
| if name_prefix is None: | ||||||
| name_prefix = 'api_{}'.format( | ||||||
| cls.__name__.replace('Resource', '').lower() | ||||||
| ) | ||||||
| name_prefix = f"api_{cls.__name__.replace('Resource', '').lower()}" | ||||||
|
Comment on lines
-108
to
+103
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||||||
|
|
||||||
| name_prefix = name_prefix.rstrip('_') | ||||||
| return '_'.join([name_prefix, name]) | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,11 +46,7 @@ def is_debug(self): | |
| return current_app.debug | ||
|
|
||
| def build_response(self, data, status=OK): | ||
| if status == NO_CONTENT: | ||
| # Avoid crashing the client when it tries to parse nonexisting JSON. | ||
| content_type = 'text/plain' | ||
| else: | ||
| content_type = 'application/json' | ||
| content_type = 'text/plain' if status == NO_CONTENT else 'application/json' | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
This removes the following comments ( why? ): |
||
| return make_response(data, status, { | ||
| 'Content-Type': content_type, | ||
| }) | ||
|
|
@@ -74,9 +70,7 @@ def build_endpoint_name(cls, name, endpoint_prefix=None): | |
| :rtype: string | ||
| """ | ||
| if endpoint_prefix is None: | ||
| endpoint_prefix = 'api_{}'.format( | ||
| cls.__name__.replace('Resource', '').lower() | ||
| ) | ||
| endpoint_prefix = f"api_{cls.__name__.replace('Resource', '').lower()}" | ||
|
Comment on lines
-77
to
+73
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
|
||
| endpoint_prefix = endpoint_prefix.rstrip('_') | ||
| return '_'.join([endpoint_prefix, name]) | ||
|
|
@@ -111,8 +105,8 @@ def add_url_rules(cls, app, rule_prefix, endpoint_prefix=None): | |
| methods=methods | ||
| ) | ||
| app.add_url_rule( | ||
| rule_prefix + '<pk>/', | ||
| f'{rule_prefix}<pk>/', | ||
| endpoint=cls.build_endpoint_name('detail', endpoint_prefix), | ||
| view_func=cls.as_detail(), | ||
| methods=methods | ||
| methods=methods, | ||
|
Comment on lines
-114
to
+111
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -116,11 +116,7 @@ def lookup_data(self, lookup, data): | |
| if callable(value) and not hasattr(value, 'db_manager'): | ||
| value = value() | ||
|
|
||
| if not remaining_lookup: | ||
| return value | ||
|
|
||
| # There's more to lookup, so dive in recursively. | ||
| return self.lookup_data(remaining_lookup, value) | ||
| return self.lookup_data(remaining_lookup, value) if remaining_lookup else value | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
This removes the following comments ( why? ): |
||
|
|
||
|
|
||
| class SubPreparer(FieldsPreparer): | ||
|
|
@@ -234,9 +230,4 @@ def prepare(self, data): | |
|
|
||
| Returns a list of data as the response. | ||
| """ | ||
| result = [] | ||
|
|
||
| for item in self.get_inner_data(data): | ||
| result.append(self.preparer.prepare(item)) | ||
|
|
||
| return result | ||
| return [self.preparer.prepare(item) for item in self.get_inner_data(data)] | ||
|
Comment on lines
-237
to
+233
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,13 +29,8 @@ def _wrapper(request): | |
| return _wrapper | ||
|
|
||
| def build_response(self, data, status=OK): | ||
| if status == NO_CONTENT: | ||
| # Avoid crashing the client when it tries to parse nonexisting JSON. | ||
| content_type = 'text/plain' | ||
| else: | ||
| content_type = 'application/json' | ||
| resp = Response(data, status_code=status, content_type=content_type) | ||
| return resp | ||
| content_type = 'text/plain' if status == NO_CONTENT else 'application/json' | ||
| return Response(data, status_code=status, content_type=content_type) | ||
|
Comment on lines
-32
to
+33
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
This removes the following comments ( why? ): |
||
|
|
||
| @classmethod | ||
| def build_routename(cls, name, routename_prefix=None): | ||
|
|
@@ -56,9 +51,7 @@ def build_routename(cls, name, routename_prefix=None): | |
| :rtype: string | ||
| """ | ||
| if routename_prefix is None: | ||
| routename_prefix = 'api_{}'.format( | ||
| cls.__name__.replace('Resource', '').lower() | ||
| ) | ||
| routename_prefix = f"api_{cls.__name__.replace('Resource', '').lower()}" | ||
|
Comment on lines
-59
to
+54
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
|
||
| routename_prefix = routename_prefix.rstrip('_') | ||
| return '_'.join([routename_prefix, name]) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -270,14 +270,12 @@ def handle(self, endpoint, *args, **kwargs): | |
| try: | ||
| # Use ``.get()`` so we can also dodge potentially incorrect | ||
| # ``endpoint`` errors as well. | ||
| if not method in self.http_methods.get(endpoint, {}): | ||
| if method not in self.http_methods.get(endpoint, {}): | ||
| raise MethodNotImplemented( | ||
| "Unsupported method '{}' for {} endpoint.".format( | ||
| method, | ||
| endpoint | ||
| ) | ||
| f"Unsupported method '{method}' for {endpoint} endpoint." | ||
| ) | ||
|
|
||
|
Comment on lines
-273
to
277
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
|
||
| if not self.is_authenticated(): | ||
| raise Unauthorized() | ||
|
|
||
|
|
@@ -340,10 +338,7 @@ def deserialize_list(self, body): | |
|
|
||
| :returns: The deserialized body or an empty ``list`` | ||
| """ | ||
| if body: | ||
| return self.serializer.deserialize(body) | ||
|
|
||
| return [] | ||
| return self.serializer.deserialize(body) if body else [] | ||
|
Comment on lines
-343
to
+341
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
|
||
| def deserialize_detail(self, body): | ||
| """ | ||
|
|
@@ -354,10 +349,7 @@ def deserialize_detail(self, body): | |
|
|
||
| :returns: The deserialized body or an empty ``dict`` | ||
| """ | ||
| if body: | ||
| return self.serializer.deserialize(body) | ||
|
|
||
| return {} | ||
| return self.serializer.deserialize(body) if body else {} | ||
|
Comment on lines
-357
to
+352
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
|
||
| def serialize(self, method, endpoint, data): | ||
| """ | ||
|
|
@@ -402,10 +394,11 @@ def serialize_list(self, data): | |
|
|
||
| # Check for a ``Data``-like object. We should assume ``True`` (all | ||
| # data gets prepared) unless it's explicitly marked as not. | ||
| if not getattr(data, 'should_prepare', True): | ||
| prepped_data = data.value | ||
| else: | ||
| prepped_data = [self.prepare(item) for item in data] | ||
| prepped_data = ( | ||
| [self.prepare(item) for item in data] | ||
| if getattr(data, 'should_prepare', True) | ||
| else data.value | ||
| ) | ||
|
Comment on lines
-405
to
+401
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
|
||
| final_data = self.wrap_list_response(prepped_data) | ||
| return self.serializer.serialize(final_data) | ||
|
|
@@ -425,10 +418,11 @@ def serialize_detail(self, data): | |
|
|
||
| # Check for a ``Data``-like object. We should assume ``True`` (all | ||
| # data gets prepared) unless it's explicitly marked as not. | ||
| if not getattr(data, 'should_prepare', True): | ||
| prepped_data = data.value | ||
| else: | ||
| prepped_data = self.prepare(data) | ||
| prepped_data = ( | ||
| self.prepare(data) | ||
| if getattr(data, 'should_prepare', True) | ||
| else data.value | ||
| ) | ||
|
Comment on lines
-428
to
+425
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
|
||
| return self.serializer.serialize(prepped_data) | ||
|
|
||
|
|
@@ -479,10 +473,7 @@ def is_authenticated(self): | |
| :returns: Whether the request is authenticated or not. | ||
| :rtype: boolean | ||
| """ | ||
| if self.request_method() == 'GET': | ||
| return True | ||
|
|
||
| return False | ||
| return self.request_method() == 'GET' | ||
|
Comment on lines
-482
to
+476
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
|
||
| # Common methods the user should implement. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,11 +24,7 @@ | |
|
|
||
| from tornado.concurrent import Future | ||
|
|
||
| if futures is None: | ||
| FUTURES = Future | ||
| else: | ||
| FUTURES = (Future, futures.Future) | ||
|
|
||
| FUTURES = Future if futures is None else (Future, futures.Future) | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lines
|
||
| is_future = lambda x: isinstance(x, FUTURES) | ||
|
|
||
|
|
||
|
|
@@ -100,22 +96,27 @@ def as_view(cls, view_type, *init_args, **init_kwargs): | |
| global _method | ||
|
|
||
| new_cls = type( | ||
| cls.__name__ + '_' + _BridgeMixin.__name__ + '_restless', | ||
| (_BridgeMixin, cls._request_handler_base_,), | ||
| f'{cls.__name__}_{_BridgeMixin.__name__}_restless', | ||
| ( | ||
| _BridgeMixin, | ||
| cls._request_handler_base_, | ||
| ), | ||
| dict( | ||
| __resource_cls__=cls, | ||
| __resource_args__=init_args, | ||
| __resource_kwargs__=init_kwargs, | ||
| __resource_view_type__=view_type) | ||
| __resource_view_type__=view_type, | ||
| ), | ||
| ) | ||
|
|
||
|
Comment on lines
-103
to
111
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
|
||
| """ | ||
| Add required http-methods to the newly created class | ||
| We need to scan through MRO to find what functions users declared, | ||
| and then add corresponding http-methods used by Tornado. | ||
| """ | ||
| bases = inspect.getmro(cls) | ||
| bases = bases[0:bases.index(Resource)-1] | ||
| bases = bases[:bases.index(Resource)-1] | ||
| for k, v in cls.http_methods[view_type].items(): | ||
| if any(v in base_cls.__dict__ for base_cls in bases): | ||
| setattr(new_cls, k.lower(), _method) | ||
|
|
@@ -129,13 +130,8 @@ def request_body(self): | |
| return self.request.body | ||
|
|
||
| def build_response(self, data, status=OK): | ||
| if status == NO_CONTENT: | ||
| # Avoid crashing the client when it tries to parse nonexisting JSON. | ||
| content_type = 'text/plain' | ||
| else: | ||
| content_type = 'application/json' | ||
| self.ref_rh.set_header("Content-Type", "{}; charset=UTF-8" | ||
| .format(content_type)) | ||
| content_type = 'text/plain' if status == NO_CONTENT else 'application/json' | ||
| self.ref_rh.set_header("Content-Type", f"{content_type}; charset=UTF-8") | ||
|
Comment on lines
-132
to
+134
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
This removes the following comments ( why? ): |
||
|
|
||
| self.ref_rh.set_status(status) | ||
| self.ref_rh.finish(data) | ||
|
|
@@ -152,14 +148,12 @@ def handle(self, endpoint, *args, **kwargs): | |
| method = self.request_method() | ||
|
|
||
| try: | ||
| if not method in self.http_methods.get(endpoint, {}): | ||
| if method not in self.http_methods.get(endpoint, {}): | ||
| raise MethodNotImplemented( | ||
| "Unsupported method '{}' for {} endpoint.".format( | ||
| method, | ||
| endpoint | ||
| ) | ||
| f"Unsupported method '{method}' for {endpoint} endpoint." | ||
| ) | ||
|
|
||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
|
||
| if not self.is_authenticated(): | ||
| raise Unauthorized() | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,7 +22,7 @@ class MoreTypesJSONEncoder(json.JSONEncoder): | |
| def default(self, data): | ||
| if isinstance(data, (datetime.datetime, datetime.date, datetime.time)): | ||
| return data.isoformat() | ||
| elif isinstance(data, decimal.Decimal) or isinstance(data, uuid.UUID): | ||
| elif isinstance(data, (decimal.Decimal, uuid.UUID)): | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
| return str(data) | ||
| else: | ||
| return super(MoreTypesJSONEncoder, self).default(data) | ||
|
|
@@ -33,8 +33,7 @@ def format_traceback(exc_info): | |
| stack = stack[:-2] | ||
| stack.extend(traceback.format_tb(exc_info[2])) | ||
| stack.extend(traceback.format_exception_only(exc_info[0], exc_info[1])) | ||
| stack_str = "Traceback (most recent call last):\n" | ||
| stack_str += "".join(stack) | ||
| stack_str = "Traceback (most recent call last):\n" + "".join(stack) | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
| # Remove the last \n | ||
| stack_str = stack_str[:-1] | ||
| return stack_str | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,10 +66,7 @@ def fake_init(self): | |
| ] | ||
|
|
||
| def is_authenticated(self): | ||
| if self.request_method() == 'DELETE' and self.endpoint == 'list': | ||
| return False | ||
|
|
||
| return True | ||
| return self.request_method() != 'DELETE' or self.endpoint != 'list' | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
|
||
| def list(self): | ||
| return self.fake_db | ||
|
|
@@ -80,7 +77,7 @@ def detail(self, pk): | |
| return item | ||
|
|
||
| # If it wasn't found in our fake DB, raise a Django-esque exception. | ||
| raise ObjectDoesNotExist("Model with pk {} not found.".format(pk)) | ||
| raise ObjectDoesNotExist(f"Model with pk {pk} not found.") | ||
|
Comment on lines
-83
to
+80
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
|
||
| def create(self): | ||
| self.fake_db.append(FakeModel( | ||
|
|
@@ -154,7 +151,7 @@ def detail(self, pk): | |
| return item | ||
|
|
||
| # If it wasn't found in our fake DB, raise a Django-esque exception. | ||
| raise Http404("Model with pk {} not found.".format(pk)) | ||
| raise Http404(f"Model with pk {pk} not found.") | ||
|
Comment on lines
-157
to
+154
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
|
||
|
|
||
| @unittest.skipIf(not settings, "Django is not available") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,10 +35,7 @@ def create(self): | |
| self.fake_db.append(self.data) | ||
|
|
||
| def is_authenticated(self): | ||
| if self.request_method() == 'DELETE': | ||
| return False | ||
|
|
||
| return True | ||
| return self.request_method() != 'DELETE' | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
|
||
|
|
||
| @unittest.skipIf(not testing, 'Pyramid is not available') | ||
|
|
||
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.
Function
UserResource.make_user_keyrefactored with the following changes:use-fstring-for-formatting)