plangrid / flask-rebar Goto Github PK

Hi there. Thanks for the great project!

I do not find a way to provide description for different fields in my request schema. Is it supported?

class GetRelationsSchema(RequestSchema):
    """
    Some request description
    """
    text = fields.String(
        required=True,
        description="some field description")

results in

I use SwaggerV3Generator.

Sorry, this may not be an issue per-se but did not know how else to reach to ask. In my project, I use Flask-Restplus and Marshmallow.

Flask-Rebar seems to solve the problem of trying to keep swagger definitions. Looking at the examples, I get the feeling that this is incompatible with Flask-Restplus, but wanted to confirm. Is that the case?
I would love to use them together!

Version: 1.5.1

It does not look like there is support for marshmallow self-referential nested fields in the swagger generation.

Schema:

class FooSchema(Schema):
    field1 = fields.Nested('self', exclude=('field1',), many=True, dump_only=True)

Error:

File ". . . flask_rebar/swagger_generation/marshmallow_to_swagger.py", line 312, in convert
    inst = nested_obj()
TypeError: 'str' object is not callable

It appears that the NestedConverter is trying to instantiate the field and doesn't handle a self reference:

# instantiate the object because the converter expects it to be
        inst = nested_obj()

When we're ready for a breaking-change release, we may want to revisit overall "namespace" type considerations (ref #86). Would be good to explore and determine if there's a nice clean way (beyond just documentation) to discourage people from importing from deeply nested e.g., "from flask_rebar.swagger_generation.swagger_generator import SwaggerV2Generator"
when they could have just done "from flask_rebar import SwaggerV2Generator"

In setup.py we currently pin to marshmallow>=2.13,<4.

As noted in comments on #43 this morning, the actual 3.0.0 release of Marshmallow introduces some breaking changes. We had tested against 3.0.0rc5 in our TravisCI setup. I just verified that 3.0.0rc6 also passes but rc7+ and the actual release of 3.0.0 do cause us to blow up.

This got conflated with a much less severe difference that I stumbled across between marshmallow 2.19 and 2.20 causing one of our tests to fail (but unlikely imho to cause any real issues) in an initially misguided and since closed PR I started that would have pinned to <=2.19 😓 Thanks @barakalon and @PratikPramanik for catching me on that one 😄 Ref #117 for a cleaner fix to that issue that doesn't mess with any pinned versions.

With the 2.19 vs 2.20 issue now handled in a more clean/compartmentalized fashion, the burning question for the next PR I need to make is: "do we consider it a breaking change if we change setup.py pin to marshmallow>=2.13,<3?" I'm of a mind that this would be more like an "unbreaking change" because anyone who simply ran pip install '.[dev]' in a clean environment since yesterday when 3.0.0 was actually released would be in a broken state, which leads me to believe correct course of action here is:

1. Release a patch version that pins to marshmallow>=2.13,<3 - potentially unbreaking anybody who broke due to 3.0.0 release of marshmallow and/or at least preventing new breakings, thereby buying us time to:
2. Actually go back and make things compatible with marshmallow 3.0.0 before:
3. re-pinning to marshmallow>=2.13,<4 once we're satisfied that we can operate with 3.*

Thoughts? If nobody has any major heartburn with this approach I'll go ahead and get a patch released tomorrow to buy us that time to deal with 3.0.0 incompatibility

One annoying thing about marshmallow is the fast that you get a dictionary instead of an object with attributes (I am bit jealous of pydantic). We started using marshmallow-objects in our project. Since we already define some custom schemas, it would be very valuable to either use this lib or make something similar. Especially since we are going toward a v2 with some breaking changes anyway.

Came across another fun Authentication use case that Rebar doesn't currently support.

OpenAPI 2 and 3 both supported AND/OR security requirements. https://swagger.io/docs/specification/authentication/#multiple

The example they give is an API that requires both an API Key and an App ID.

Writing an Authenticator that does this is simple:

from flask import request, g
from flask-rebar import HeaderAPIKeyAuthenticator, errors


class MyAuthenticator(HeaderAPIKeyAuthenticator):
    def __init__(api_key_header, app_id_header, name="myAuth")
        super(MyAuthenticator, self).__init__(api_key_header, name=name)
        self.app_id_header = app_id_header

    def authenticate(self):
        super(MyAuthenticator, self).authenticate()
        if self.app_id_header not in request.headers:
            raise errors.Unauthorized("Missing application id")
        g.authenticated_app_id = request.headers.get(self.app_id_header)


authenticator = MyAuthenticator("X-API-Key", "X-App-ID")

The correct swagger spec is also pretty simple

"securitySchemes": {
    "apiKey": {
        "type": "apiKey",
        "in": "header",
        "name": "X-API-Key"
    },
    "appID": {
       "type": "apiKey",
        "in": "header",
        "name": "X-App-ID"
    },
}

"security": [
    {"apiKey": [], "appID": []},
]

However you're basically stuck writing your own SwaggerGenerator if you want to generate such a spec since our AuthenticatorConverter only supports {name: []} SecurityRequirements and only supports one SecurityDefinition per Authenticator.

Also that {name:[]} SecurityRequirements assumption means that we can't generate valid requirements for OAuth and OpenIDConnect since those should include scope names in the list.

I'm proposing switching to Authenticators registering a AuthenticatorConverter object rather than a single method. An AuthenticatorConverter should have get_security_schemas and get_security_requirements methods that accept an Authenticator instance, and return a list of Security Scheme Objects and Security Requirement Objects respectively

https://github.com/plangrid/flask-rebar/blob/master/setup.py#L19 specifies marshmallow>=2.13. Right now this is picking up Marshmallow 2, but as soon as Marshmallow 3 final comes out, this will start picking up v3, which has several backwards-incompatible changes:

https://marshmallow.readthedocs.io/en/3.0/changelog.html

Are any updates necessary before then?

I created an authenticator for cognito, do you think that I should create a PR to merge it in Rebar or it should live it's in own package?
I want to battle test it a bit though before releasing since I am not sure of all the flows of cognito.

https://flask-rebar.readthedocs.io/en/latest/api_reference.html currently shows nothing but section headings.

I think it would be nice to be able to deprecate APIs and parameters.
See this link for reference.

The swagger_path param is used to set the location of the OpenAPI spec file. But since 1/1/2016, OpenAPI refers to the specification; Swagger refers to tooling around it (e.g. the Swagger UI). Would it make sense to rename this param? Something like openapi_path or just spec_path?

It looks like flask-rebar provides a SwaggerV2Generator but not a SwaggerV3Generator (or similar). OpenAPI Spec v3 was released last year (see https://github.com/OAI/OpenAPI-Specification/releases). Is there any interest in adding support for this to flask-rebar?

Following up on recent PRs, figured I would open an Issue for now where we can kick some ideas around on the topic of content negotiation.

A few quick thoughts to kick things off. I would characterize our options as:

1. Do nothing (ok now that's out of the way)
2. Build our own content negotiation
3. Add a dependency on some existing content negotiation library (I haven't looked deeply into these yet but found a couple like Flask-Accept, Flask-Negotiate, Flask-Negotiation)
4. Punt content-type negotiation and add some recipes/examples that demonstrate how one could integrate Flask-Rebar with things like those in item 3.

Pros/cons I see of these:

1. Duh, no good.
2. Pro: We can make it work exactly how we want. Con: Risk of reinventing the wheel
3. Pro: Citizenship badge for OSS :). Con: I like the fact that we are only dependent on the two core technologies that we are combining in Flask and Marshmallow and am hesitant to add to the Python equivalent of DLL-Hell.
4. Pro: Still good OSS citizens (and without choosing sides - allow people to stick with what they like if they already have a preference). Con: Pushes some of the DLL-hell burden to our users

If I was forced to rank these I'd probably say 2 & 4 are a virtual tie for me, followed by 3, and then 1. Definitely interested to hear thoughts from our other contributors and users.

I wanted to check what version of Swagger UI Flask-Rebar is bundling, and there seems to be no place this is documented. Looked in https://github.com/plangrid/flask-rebar/tree/master/flask_rebar/swagger_ui/static as well and it doesn't seem to be there either.

One idea could be to check in some update-swagger-ui.sh script that checks e.g. https://cdnjs.com/libraries/swagger-ui, ensures the latest version is getting bundled, and writes the version to some VERSION file in the swagger_ui/static dir.

In working to address #73 (in which some tests were broken because Travis CI used latest version of Werkzeug, which tweaked error messages and - more importantly - changed from 301 to 308 for redirect on missing trailing slash), I have stumbled across another issue.

Adjusting the trailing slash test to accept either 301 or 308, I now get Travis build errors specifically for Flask 0.10 (but not for 0.11+). It would seem that Flask 0.10 does not handle the 308 correctly, specifically, it fails to maintain the same content-type. All of which leads to a few questions that I thought might make sense to discuss here.

Biggest question: Does anyone see a need to fully support Flask 0.10 (nearly 6 years old)? I'm hard-pressed to imagine a real-world scenario in which somebody would be using Flask 0.10 and trying to use something as modern and shiny as Flask-Rebar. ;-)

Probably the simplest way for me to unblock on this issue is just to remove that from Travis CI tests. So, we wouldn't technically "require" 0.11+ but it would make it on par with pre-0.10 versions.. Not technically "required" but also not tested as part of our standard build.

Does that cause anyone heartburn? (cc @barakalon - sorry for Flask-Rebar spam today LOL, but I'm definitely interested in your opinion as the father of this library. Any additional insights into how particular pairings of Flask and Marshmallow were selected when building .travis.yml would be much appreciated!)

Since the future is ASGI (Django announced support for v3), it would be a good idea for flask-rebar to support at least Quart to provide a smoother transition.

Hi, thank you guys for all good work, I have question , I am lazy :), I am testing flask-rebar and want to use flask-marshmallow
class UserSchema(ma.Schema):
username = fields.Str()
created_at = fields.DateTime()
_links = ma.Hyperlinks({'collection': ma.URLFor("api/v1.get_users")})

This works as url, but fails to generate swagger doc, I am guessing you need to "register" URLFor and Hyperlinks , any ideas

It would be great to be able to hide some APIs if they are not meant to be public. A parameter in the handles decorator would do the trick.

At least, document this (favor use of QueryParamList or CommaSeparatedList
At best, build in native support for fields.List (prob with same behavior as QueryParamList)

This is already important for us since we started using mypy to type check our code. We have to ignore all code flask-rebar currently.

It might be worth making a more general method to handle bumping up a list of properties from the jsonschema to the parameter.

Looking over the 2.0/3.0.2 differences if we ever added support for any of the following fields, then I think they would also want to be "bumped up a level".
v2 & v3: allowEmptyValues
v3 only: deprecated, allowReserved, example, examples

Originally posted by @airstandley in #129 (comment)

I often use the many=True for Schema in the returned body, but it's not possible with the ResponseSchema class because it doesn't implement a constructor.

requirements.txt is meant to be used by applications, not libraries, and this is a library. Rebar should be using setup.py's install_requires as the single source of truth for always-required dependencies, and an extras_require field to set up additional sets of dependencies e.g. for testing. (So you'd do e.g. pip install -e .[test] to get the additional test dependencies.)

Here's an example:

• https://github.com/jab/bidict/blob/master/setup.py#L117
• https://github.com/jab/bidict/blob/master/.travis.yml#L31

https://flask-rebar.readthedocs.io/en/latest/quickstart/swagger_generation.html#default-response demonstrates overriding the default swagger generator, but mistakenly documents passing swagger_generator to the Rebar initializer (which does not accept this argument):

rebar = Rebar(swagger_generator=generator)

It should instead document passing this to the Registry initializer, which does accept this argument.

I just discovered flask-rebar and I think it could very well become the new standard for REST with flask. The documentation is very well made, but a more complete "real-life" example/template would help a lot to get started. I am creating one for my project that i could share, but I don't know all the best practices for the framework.
Keep up the good work, cheers!

It is pretty common to use dependency injection with Flask-Injector. Mainly to inject a service in the controller for better testing. Maybe it could be nice to integrate some kind of similar system. Currently I create my service at the top of the controller file and I patch it in the test.

A simple parentheses omission in schema instantiation like below (see the comment in the request_body_schema line) can cause a gnarly swagger generation error message:

Code:

from marshmallow import Schema
from flask_rebar import Rebar

class MySchema(Schema):
    ...

rebar = Rebar()
registry = rebar.create_handler_register(...)
@registry.handles(
    rule='/endpoint',
    method='POST',
    request_body_schema=MySchema   # Should be MySchema()
):
    ...

Error message:
flask_rebar.swagger_generation.marshmallow_to_swagger.UnregisteredType: No registered type found in method resolution order: (<class 'marshmallow.schema.SchemaMeta'>, <class 'type'>, <class 'object'>)
Registered types: [<class 'marshmallow.fields.Boolean'>, <class 'marshmallow.fields.
Date'>, ...]

I spent ~30 minutes debugging a similar error in my app. It would be great if rebar could suggest to the user to check if they missed any parentheses.

Rebar has the ability to set a default_authenticator that is applied to all handlers in a registry.

It is a fairly common pattern to support multiple styles of authentication, for example one might allow a user to login and pass a session token, or instead allow a client to pass an api key.

I'm proposing we make it possible to define both the authenticator for a handler, and the default_authenticator as a list of Authenticators, and change the wrap_handler method so that if any of these Authenticators passes then the request is considered authenticated.

USE_DEFAULT is defined in flask_rebar.authenticators but its usage has been expanded so it probably belongs in a more generic utility module

The next major release should kill Python 2 support, and we should start preparing to release that soon. We're down to the wire on end-of-life for Python 2.

I realized that nested Schemas are displayed as {}.

{
    "device_id": "string",
    "name": "string",
    "parameters": [
      {}
    ],
}

app.py:

from flask_rebar import Rebar
from flask import Flask
from marshmallow import Schema, fields

app = Flask(__name__)
rebar = Rebar()
registry = rebar.create_handler_registry()


class Foo(Schema):
    foo = fields.Str(required=True)


@registry.handles(
    rule='/foos/',
    marshal_schema=Foo(many=True),
)
def get_foos():
    return [{'foo': 'foo1'}, {'foo': 'foo2'}]


rebar.init_app(app)

Resulting spec:

{
  "consumes": [
    "application/json"
  ], 
  "definitions": {
    "Error": {
      "properties": {
        "errors": {
          "type": "object"
        }, 
        "message": {
          "type": "string"
        }
      }, 
      "required": [
        "message"
      ], 
      "title": "Error", 
      "type": "object"
    }, 
    "Foo": {
      "properties": {
        "foo": {
          "type": "string"
        }
      }, 
      "required": [
        "foo"
      ], 
      "title": "Foo", 
      "type": "object"
    }
  }, 
  "host": "localhost:5000", 
  "info": {
    "description": "", 
    "title": "My API", 
    "version": "1.0.0"
  }, 
  "paths": {
    "/foos/": {
      "get": {
        "operationId": "get_foos", 
        "responses": {
          "200": {
            "description": "Foo", 
            "schema": {
              "$ref": "#/definitions/Foo"
            }
          }, 
          "default": {
            "description": "Error", 
            "schema": {
              "$ref": "#/definitions/Error"
            }
          }
        }
      }
    }
  }, 
  "produces": [
    "application/json"
  ], 
  "schemes": [
    "http"
  ], 
  "securityDefinitions": {}, 
  "swagger": "2.0"
}

GET /foos/ should be returning an array of Foo's, not a single Foo.

Using classes for view functions in Flask is a common patten (see flask-restful and flask-restplus).

Using classes is currently possible with Flask-Rebar, but we should add whatever is needed to make it really easy to do this.

To better match request_body_schema. (First deprecate but continue to support marshal_schema as an alias of response_body_schema until Rebar v2.)

See marshmallow-code/apispec#84 for an example of a competing library adding this feature.

SwaggerV2Generator produces application/vnd.plangrid+json by default:

This seemed unusual and after looking a bit I wasn't able to find any clues as to why this doesn't use application/json instead. Any interest in switching to application/json?

Thanks for your consideration and for maintaining flask-rebar. After looking at several options (flask-apispec, flasgger, flask-restplus), this seems like the best.

Since this came up in the discussion, would it be a good idea to just jump into the deep and make add a test to ensure pep8 compliance going forward?
Do people have a strong opinion about pep8 settings or should we just go with the vanilla version for now?

Thoughts?

Swagger supports example response payloads: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#response-object

We should add a mechanism for supporting this.

Along these same lines, we should aim to make the swagger spec easily overridable/extendable in all cases. Automatic generation is great until it doesnt work, and we should always have a fallback.

Today we discussed creating our own class of errors because we wanted to have uniforms errors with a machine parsable type. Though we could add it just fine with the additional_data field, it is a bit painful to enforce it on every error thrown. The problem is that we can't currently modify the default errors that Flask-Rebar raise in the case of an invalid schema for example. I don't have a clear answer as to how we should handle this, but I figured I could ask if someone had an idea as to how we could do this.

Hi Rebar maintainers and community, just wondering, how are y'all coping with marshmallow-code/marshmallow#1295?

Are we all just reinventing this wheel slightly differently in our own code? Settling for snake_case fields in our JSON or (PEP-8-violating) camelCase fields in our Python? Something else? Thanks in advance for any helpful discussion 😅

Out of the box, Flask sets up nice automatic handling of trailing slash redirects. So if you have a handler mapped to a URL like /swagger/ui/ (such as the one Flask-Rebar sets up for you), but the user makes a request like /swagger/ui (note no trailing slash), then normally, Flask serves the user a redirect to /swagger/ui/ without your having to do anything extra, whether or not app.debug is True.

However, Flask-Rebar's _init_error_handling is interfering with this in the case that app.debug is False. I hit this after writing a function start_devserver(debug=False). Before calling app.run(debug=debug), it prints out the string f"* Swagger UI available at http://localhost:5000/{registry.prefix}/{registry.swagger_ui_path}". With debug=False, clicking the resulting link would result in a 500 error and the following stack trace:

[2018-10-26 15:57:55,690] ERROR in app: Exception on /swagger/ui [GET]
Traceback (most recent call last):
  File "/private/tmp/testvenv/lib/python3.7/site-packages/werkzeug/routing.py", line 1538, in match
    rv = rule.match(path, method)
  File "/private/tmp/testvenv/lib/python3.7/site-packages/werkzeug/routing.py", line 776, in match
    raise RequestSlash()
werkzeug.routing.RequestSlash

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/private/tmp/testvenv/lib/python3.7/site-packages/flask/app.py", line 1813, in full_dispatch_request
    rv = self.dispatch_request()
  File "/private/tmp/testvenv/lib/python3.7/site-packages/flask/app.py", line 1791, in dispatch_request
    self.raise_routing_exception(req)
  File "/private/tmp/testvenv/lib/python3.7/site-packages/flask/app.py", line 1774, in raise_routing_exception
    raise request.routing_exception
  File "/private/tmp/testvenv/lib/python3.7/site-packages/flask/ctx.py", line 336, in match_request
    self.url_adapter.match(return_rule=True)
  File "/private/tmp/testvenv/lib/python3.7/site-packages/werkzeug/routing.py", line 1542, in match
    safe='/:|+') + '/', query_args))
werkzeug.routing.RequestRedirect: 301 Moved Permanently: None
127.0.0.1 - - [26/Oct/2018 15:57:55] "GET /swagger/ui HTTP/1.1" 500 -

It looks like the logic here is the culprit:

It's some combination of:

1. catching too-broad an exception type (as warned against in pallets/flask#2948 (comment))
2. clobbering (and/or forgetting to re-add) the special handling for RequestRedirect that is done in https://github.com/pallets/flask/blob/70b45094ab3d62b919a84f8fb8376e6614a98dd3/flask/app.py#L1771)
3. doing different things when app.debug is True or False for exceptions like RequestRedirect that should not care.

I also noticed that a nested function named handle_werkzeug_http_error() is defined twice inside _init_error_handling(), which seems like unintentional copy pasta.

I don't see anything about tags in the docs, nor any signs of support from https://github.com/plangrid/flask-rebar/search?q=tags&type=Code

Is there support for tags, or if not would you want to add it?

Ref: https://swagger.io/docs/specification/2-0/grouping-operations-with-tags/

Thanks!

How can I validate length of CommaSeparatedList? This is not working:

from marshmallow import fields, validate, Schema
from flask_rebar.validation import CommaSeparatedList

class Dummy(Schema):
    extent = CommaSeparatedList(
        fields.Float(),
        required=True,
        validates=validate.Length(equal=4)
    )

Appears to be due to pallets/flask#2986

At first glance it looks to me like we're no longer getting an "error" on redirect (and we end up with default mimetype of text/html in the 301 response).

Could use another set of eyes (particularly people more intimately familiar with the internals of Flask than I ;) ) to help understand what's happening and how we can cleanly address. To reproduce, simply update Flask from 1.0.2 to 1.0.3 and run our unit tests.

SwaggerV2Generator currently defaults host to "swag.com":

Since the Flask-Rebar users I'm supporting all use Rebar's (awesome) serverless generation feature to generate an OpenAPI spec build artifact for their APIs, they all end up with host: swag.com in their api.yml's. Since this spec file then goes on to become the input for Swagger-Codegen-generated e.g. TypeScript clients, the default basePath of the generated clients becomes "https://swag.com". So users inevitably have to add the following boilerplate to get a working client integrated into their Flask-served UIs:

import * as Client from "../../gensrc/client"
const Api = new Client.WhoamiApi({basePath: location.origin})

As with #61, I'm wondering if Rebar could be using a better default value for "host" that would reduce this boilerplate for users, or if anyone has any other recommendations here. Thanks!

In marshmallow when using the same schema in multiple places, I can use partial to skip required validation. One typical use case of this is on POST (i.e. creating a new object) I want all fields to be required, however on PATCH (i.e. updating an existing object) I want all fields to be not required, and only update the fields specified in the payload. E.g.,

class UserSchema(Schema):
    name = fields.String(required=True)
    age = fields.Integer(required=True)

user_schema = UserSchema()
user_update_schema = UserSchema(partial=True)

I then able to use request_body_schema=user_schema in the POST endpoint and request_body_schema=user_update_schema in the PATCH endpoint and it's working perfectly in Flask.

However in generated swagger definition (and swagger client), the partial keyword has no effect, and thus all fields are still required in the PATCH endpoint. The relevant code does not seem to look at partial at all. Is this a good feature to support? I can create a PR for this.

PS. If we concern about POST and PATCH schema have the same name (UserSchema) and this mapped to the same model in swagger, then I could just do this:

class UserUpdateSchema(UserSchema):
    pass

user_update_schema = UserUpdateSchema(partial=True)

Related to #12

I've been thinking about this a bit, and I'd love to be able to extract some of the swagger properties from the doc-strings as well as from magic attributes (like __swagger_title__).
(There is some precedent for this as we already support pulling the description of a path from the docstring.)

This might end up being un doable since enforcing formats in comments in notoriously hard.
However I think it would be fantastic if

class UserResponseSchema(ResponseSchema):
    """
    User:
        A user's details.

    Example:
         {
             "first_name": "Bruce",
             "last_name": "Lee",
             "email": "[email protected]"
        }
    """
    first_name = fields.String()
    last_name = fields.String()
    email = fields.Email()

Rendered to a ResponseObject of

{
    "User": {
        "description": "A user's details.",
        "schema": {"$ref": "#/definitions/UserResponseSchema"},
        "examples": {
            "application/json": {
               "first_name": "Bruce",
               "last_name": "Lee",
               "email": "[email protected]"
            }
        }
    }
}

Interested in other's thoughts.

Looks like info for the last few patch releases is missing from https://github.com/plangrid/flask-rebar/releases and https://github.com/plangrid/flask-rebar/blob/master/CHANGELOG.rst.