Frameworks passing #394

dutradda · 2017-02-01T18:43:34Z

Hello guys,

I am doing a PR in master because I rewrite the frameworks branch. Now all testing are passing and flask are full uncouple.

@rafaelcaricio this just a API proposal

dutradda · 2017-02-01T18:50:21Z

the tests envs with -dev option are breaking and I dont know why..
the error says that a site-packages/six-1.10.0.dist-info/METADATA is missing.
In my machine I created this files by hand and all tests are passing.

coveralls · 2017-02-01T18:53:24Z

Coverage remained the same at 100.0% when pulling 7671ea3 on dutradda:frameworks_passing into 8e10a77 on zalando:master.

rafaelcaricio · 2017-02-02T10:24:48Z

@dutradda Muito obrigado pelo seu PullRequest! I will start reviewing it ASAP.

About the failing tests, we can take a look at this part after the code review.

dutradda · 2017-02-14T02:27:27Z

@rafaelcaricio any progress on this?

rafaelcaricio · 2017-02-14T12:42:13Z

Hi @dutradda, It is a big PR. But I am trying to find time this week to do the review. It cleaned up some checks that I think do not make sense from the codacy bot and left the ones I think should be tackled. Could you please start by those two?

rafaelcaricio · 2017-02-14T12:42:52Z

Note to self: I have to fix up the configuration of the codacy bot.

rafaelcaricio · 2017-02-14T12:44:06Z

connexion/apis/abstract.py

@@ -153,7 +142,26 @@ def __init__(self, specification, base_url=None, arguments=None,
        self.add_paths()

        if auth_all_paths:
-            self.add_auth_on_not_found()
+            self.add_auth_on_not_found(self.security, self.security_definitions)


What do you think about in the original method to add *args, **kwargs just to signal that it might have arguments in the subclasses?

coveralls · 2017-02-14T22:08:13Z

Coverage remained the same at 100.0% when pulling ad8a3b5 on dutradda:frameworks_passing into fe6a625 on zalando:master.

coveralls · 2017-02-14T22:08:13Z

Coverage remained the same at 100.0% when pulling ad8a3b5 on dutradda:frameworks_passing into fe6a625 on zalando:master.

rafaelcaricio

@dutradda I have done a quick review. Please take a look on what I have commented. Thank you for working on this.

rafaelcaricio · 2017-02-15T14:03:20Z

connexion/apis/abstract.py

@@ -47,22 +46,17 @@ def compatibility_layer(spec):
    return spec


-def canonical_base_url(base_path):
+@six.add_metaclass(abc.ABCMeta)
+class AbstractApi(object):


I would suggest to change the name to AbstractAPI.

rafaelcaricio · 2017-02-15T14:04:41Z

connexion/apis/abstract.py

    """
-    Make given "basePath" a canonical base URL which can be prepended to paths starting with "/".
+    Single Abstract API


Could you please describe in high level what is this class responsible for? "Defines an abstract interface for ..."

rafaelcaricio · 2017-02-15T15:43:07Z

connexion/apis/abstract.py

+
+    @abc.abstractmethod
+    def add_swagger_json(self):
+        """"""


Would be nice to have documented here what the implementations of this method are expected to do.

rafaelcaricio · 2017-02-15T15:43:59Z

connexion/apis/abstract.py

+
+    @abc.abstractmethod
+    def add_auth_on_not_found(self, security, security_definitions):
+        """"""


Would be nice to have documented here what the implementations of this method are expected to do.

rafaelcaricio · 2017-02-15T15:44:03Z

connexion/apis/abstract.py

+
+    @abc.abstractmethod
+    def add_swagger_ui(self):
+        """"""


Would be nice to have documented here what the implementations of this method are expected to do.

rafaelcaricio · 2017-02-15T15:55:10Z

connexion/apps/abstract.py

-            http_server.serve_forever()
-        else:
-            raise Exception('Server %s not recognized', self.server)
+        """"""


Would be nice to have documented here what the implementations of this method are expected to do.

rafaelcaricio · 2017-02-15T15:57:13Z

connexion/apps/flask_app.py

+
+
+class FlaskJSONEncoder(json.JSONEncoder):
+    def default(self, o):


You can ignore this.

rafaelcaricio · 2017-02-15T16:01:11Z

connexion/decorators/response.py

@@ -44,10 +43,10 @@ def validate_response(self, data, status_code, headers):
            try:
                # For cases of custom encoders, we need to encode and decode to
                # transform to the actual types that are going to be returned.
-                data = json.dumps(data)
-                data = json.loads(data)
+                data = data.replace(b'\\"', b'"')


We need to handle encoding properly. Could you please elaborate on what is the real problem here?

rafaelcaricio · 2017-02-15T16:03:00Z

connexion/decorators/response.py

-                data = json.dumps(data)
-                data = json.loads(data)
+                data = data.replace(b'\\"', b'"')
+                data = self.operation.api.jsonifier.loads(data)


Probably we should add a top level method in the self.operation or api to handle this better. This something.calling.another.internal.thing will lead us to have a headache later.

rafaelcaricio · 2017-02-15T16:05:58Z

connexion/decorators/response.py

            except NonConformingResponseHeaders as e:
-                return problem(500, e.reason, e.message)
+                response = problem(500, e.reason, e.message)


What about:

except (NonConformingResponseBody, NonConformingResponseHeaders) as e: ...

?

rafaelcaricio · 2017-02-17T14:01:39Z

This work is related to #380

LappleApple · 2017-03-14T12:51:16Z

Hi @dutradda, wondering how things are going with your work on Connexion. It's pretty exciting for us to have Mozilla contributing, and I'd like to help promote your work once it's complete. -- Zalando OSS Evangelist

dutradda · 2017-03-14T19:52:56Z

@rafaelcaricio I did the changes you request
@LappleApple Thanks for appreciating! I am exciting too! This is my first big contribution to a free software project.
Thanks all of you guys

coveralls · 2017-03-14T20:03:29Z

Coverage remained the same at 100.0% when pulling 2699c36 on dutradda:frameworks_passing into e9db7fe on zalando:master.

coveralls · 2017-03-16T16:32:52Z

Coverage remained the same at 100.0% when pulling 225eeae on dutradda:frameworks_passing into e9db7fe on zalando:master.

coveralls · 2017-03-16T17:03:42Z

Coverage remained the same at 100.0% when pulling 220d93a on dutradda:frameworks_passing into d1df8f6 on zalando:master.

coveralls · 2017-03-16T17:17:52Z

Coverage remained the same at 100.0% when pulling 35cd831 on dutradda:frameworks_passing into d1df8f6 on zalando:master.

coveralls · 2017-03-16T17:17:52Z

Coverage remained the same at 100.0% when pulling 35cd831 on dutradda:frameworks_passing into d1df8f6 on zalando:master.

coveralls · 2017-03-16T17:17:52Z

Coverage remained the same at 100.0% when pulling 35cd831 on dutradda:frameworks_passing into d1df8f6 on zalando:master.

coveralls · 2017-03-16T17:17:52Z

Coverage remained the same at 100.0% when pulling 35cd831 on dutradda:frameworks_passing into d1df8f6 on zalando:master.

coveralls · 2017-03-16T17:33:26Z

Coverage remained the same at 100.0% when pulling 1425f53 on dutradda:frameworks_passing into d1df8f6 on zalando:master.

…modules. Fixed the tests

removed test_decorators and test_parameter (this test is useless now); removed the request/response containers and add new request response classes; created a abstract api class and a api flask class; derived classes will implements the get_response/get_request methods that will convert framework req/resp types to connexion req/resp types; moved the jsonifier from produces to flask api; created a abstract app class and a app flask class; changed all validators to use the ConnexionRequest instead flask request; changed the problem function to generate a ConnexionRequest; created a new user variables container called context (this is a property of ConnexionRequest). this will be passed as kwargs to all operations functions; this context is used on authentication; fixed all tests to new API; some changes that I did may not be documented in this commit.

Documented some AbstractAPI and AbstractApp methods; Added parameters on some abstract methods; Removed useless code from ResponseValidator; Created a wrapper for api.jsonifier.loads on Operation class; Merged with master.

…old classes

coveralls · 2017-04-04T11:43:55Z

Coverage remained the same at 100.0% when pulling 65bfe5a on dutradda:frameworks_passing into 711bbf0 on zalando:master.

coveralls · 2017-04-04T11:43:56Z

Coverage remained the same at 100.0% when pulling 65bfe5a on dutradda:frameworks_passing into 711bbf0 on zalando:master.

coveralls · 2017-04-04T11:43:56Z

Coverage remained the same at 100.0% when pulling 65bfe5a on dutradda:frameworks_passing into 711bbf0 on zalando:master.

coveralls · 2017-04-04T13:20:59Z

Coverage remained the same at 100.0% when pulling 719f685 on dutradda:frameworks_passing into 711bbf0 on zalando:master.

coveralls · 2017-04-04T13:20:59Z

Coverage remained the same at 100.0% when pulling 719f685 on dutradda:frameworks_passing into 711bbf0 on zalando:master.

coveralls · 2017-04-04T13:34:50Z

Coverage remained the same at 100.0% when pulling 56e6197 on dutradda:frameworks_passing into 711bbf0 on zalando:master.

coveralls · 2017-04-04T13:34:50Z

Coverage remained the same at 100.0% when pulling 56e6197 on dutradda:frameworks_passing into 711bbf0 on zalando:master.

coveralls · 2017-04-04T13:34:50Z

Coverage remained the same at 100.0% when pulling 56e6197 on dutradda:frameworks_passing into 711bbf0 on zalando:master.

coveralls · 2017-04-04T13:34:51Z

Coverage remained the same at 100.0% when pulling 56e6197 on dutradda:frameworks_passing into 711bbf0 on zalando:master.

rafaelcaricio · 2017-04-04T13:42:59Z

👍

dutradda · 2017-04-04T14:04:44Z

@rafaelcaricio today is my birthday, is a great gift for me!
I changed the setup.py to install flask just optionally, so would be interesting to warn in README or something like this

rafaelcaricio · 2017-04-04T14:06:57Z

@dutradda Well, feliz aniversário! 🎂 🎈

Well thought, I will add a note in the README for that. 😉

rafaelcaricio reviewed Feb 14, 2017

View reviewed changes

dutradda force-pushed the frameworks_passing branch from ce21065 to ad8a3b5 Compare February 14, 2017 21:55

rafaelcaricio suggested changes Feb 15, 2017

View reviewed changes

rafaelcaricio added the waiting contributor feedback label Feb 17, 2017

rafaelcaricio mentioned this pull request Feb 17, 2017

Frameworks #386

Closed

tisto mentioned this pull request Feb 27, 2017

Keeping plone.restapi and plone server API in sync (was: Doubt about schema / content type serialization) plone/plone.restapi#157

Open

dutradda force-pushed the frameworks_passing branch from ad8a3b5 to 2699c36 Compare March 14, 2017 19:51

rafaelcaricio removed the waiting contributor feedback label Mar 16, 2017

dutradda force-pushed the frameworks_passing branch from 225eeae to 1de525f Compare March 16, 2017 16:32

dutradda force-pushed the frameworks_passing branch from 1de525f to 220d93a Compare March 16, 2017 16:51

dutradda force-pushed the frameworks_passing branch from 220d93a to 35cd831 Compare March 16, 2017 17:05

dutradda added 13 commits April 4, 2017 08:20

Uncouple flask from App and Api. Created Abstract classes for theses …

a7af62f

…modules. Fixed the tests

uncouple flask utils functions from utils module

7c87126

Uncouple flask from operation

3067315

fixed missing method parameters and trailling spaces

5cc848f

rebased with master

e7a75d5

Renamed Api classes to API;

96ba88a

Documented some AbstractAPI and AbstractApp methods; Added parameters on some abstract methods; Removed useless code from ResponseValidator; Created a wrapper for api.jsonifier.loads on Operation class; Merged with master.

did changes requested by isort and flake8

3952ea2

rebased with master

7552289

did flask a optional dependency

fb7f986

did changes requested by isort-check

2ddebfd

rebased with master and added backward compatibility for Api and App …

65bfe5a

…old classes

dutradda force-pushed the frameworks_passing branch from 02ce610 to 65bfe5a Compare April 4, 2017 11:32

Use lowecase for local variables

719f685

Keep README using old connexion.App class

56e6197

rafaelcaricio merged commit 53908d3 into spec-first:master Apr 4, 2017

mvalkon mentioned this pull request Aug 22, 2017

Support for sanic/starlette framework or async/await #496

Closed



		class FlaskJSONEncoder(json.JSONEncoder):
		def default(self, o):

Frameworks passing #394

Frameworks passing #394

Conversation

dutradda commented Feb 1, 2017

dutradda commented Feb 1, 2017

coveralls commented Feb 1, 2017

rafaelcaricio commented Feb 2, 2017

dutradda commented Feb 14, 2017

rafaelcaricio commented Feb 14, 2017

rafaelcaricio commented Feb 14, 2017

Choose a reason for hiding this comment

coveralls commented Feb 14, 2017 • edited

coveralls commented Feb 14, 2017

rafaelcaricio left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rafaelcaricio commented Feb 17, 2017

LappleApple commented Mar 14, 2017

dutradda commented Mar 14, 2017

coveralls commented Mar 14, 2017 • edited

coveralls commented Mar 16, 2017 • edited

coveralls commented Mar 16, 2017 • edited

coveralls commented Mar 16, 2017

coveralls commented Mar 16, 2017

coveralls commented Mar 16, 2017

coveralls commented Mar 16, 2017

coveralls commented Mar 16, 2017 • edited

coveralls commented Apr 4, 2017

coveralls commented Apr 4, 2017

coveralls commented Apr 4, 2017 • edited

coveralls commented Apr 4, 2017 • edited

coveralls commented Apr 4, 2017 • edited

coveralls commented Apr 4, 2017

coveralls commented Apr 4, 2017

coveralls commented Apr 4, 2017

coveralls commented Apr 4, 2017

rafaelcaricio commented Apr 4, 2017

dutradda commented Apr 4, 2017

rafaelcaricio commented Apr 4, 2017

coveralls commented Feb 14, 2017 •

edited

coveralls commented Mar 14, 2017 •

edited

coveralls commented Mar 16, 2017 •

edited

coveralls commented Mar 16, 2017 •

edited

coveralls commented Mar 16, 2017 •

edited

coveralls commented Apr 4, 2017 •

edited

coveralls commented Apr 4, 2017 •

edited

coveralls commented Apr 4, 2017 •

edited