eabglobal / minik Goto Github PK

What

• Configure sphinx as the framework of choice for documentation purposes.
• As part of the configuration process, refactor the documentation in the README in a more logical manner.
• Update README to include a section on how to build / update the docs
• Setup the gh-pages branch to host the documentation.

How

Install sphinx and run the initial configuration. Once the initial config is done, streamline and simplify the README file of the project by moving the auxiliary info to it's own page in the sphinx docs.

WHAT
Update minik to support CORS. This effort could be two part process. Enhance the router to enable a basic CORS config as in

@app.get('/supports-cors', cors=True)

Adding this flag should:

• Automatically add an OPTIONS method to support preflighting requests.
• Set the Access-Control-Allow-Origin: * in the header
• Set Access-Control-Allow-Methods to the set of HTTP methods called out in thew view function.
• Access-Control-Allow-Headers: Content-Type,X-Amz-Date,Authorization, X-Api-Key,X-Amz-Security-Token.

The second request is to enhance the request response workflow to support the return of a Response instance instead of just a json object. In this way a consumer can have more freedom to update the headers and body of the response.

@app.route('/users', methods=['GET', 'OPTIONS'])
def support_custom_response():
    method = app.request.method
    if method == 'OPTIONS':
        headers = {
            'Access-Control-Allow-Method': 'GET,OPTIONS',
            'Access-Control-Allow-Origin': ','.join(_ALLOWED_ORIGINS),
            'Access-Control-Allow-Headers': 'X-Some-Header',
        }
        origin = app.request.headers.get('origin', '')
        if origin in _ALLOWED_ORIGINS:
            headers.update({'Access-Control-Allow-Origin': origin})
        return Response(
            body=None,
            headers=headers,
        )

Defaulting mutable values in python is NOT a good practice.

https://github.com/eabglobal/minik/blob/master/minik/core.py#L142

Remove the default value of the headers in the core.py.

This is the side effect caused by defaulting values in the function definition.

>>> def test(headers={}):
...   if 'thing' in headers:
...     headers['thing'] += 1
...   else:
...     headers['thing'] = 0
...   print(headers)
... 
>>> test()
{'thing': 0}
>>> test()
{'thing': 1}
>>> test()
{'thing': 2}

WHAT
Right now minik is very tightly coupled with the way in which we parsed the API gateway event. The Request is built in the core based on the values of the API gateway object. It would be nice if a developer could specify a factory that would return a request based on a given event. The event could come from the API gateway or from a load balancer or from whatever.

As long as the factory returns a valid Request object, the middleware can be used to configure how minik should parse an event.

Proposed syntax:

from minik.builders import APIGatewayRequestBuilder

app = Minik()
app.set_request_builder(APIGatewayRequestBuilder)

The APIGatewayRequestBuilder would be the default builder. However, an API developer can have the option to create his own request builder.

What

Minik needs to have the ability to include the HTTP method as a way to define the view used for routing purposes. The business need can be defined by the following example:

   @app.route('/events/{event_type}', methods=['POST', 'PUT'])
    def post_event_view(event_type):
        # Update the event object.
        return {'complete': True}

   @app.route('/events/{event_type}', methods=['GET'])
    def get_event_view(event_type):
        return {'data': ['MD Grand Fondo', 'San Diego Sparta']}

As seen above, the same /events/{event_type} route is used, but for different HTTP methods, it has different views.

Why?

The ability of the framework to evolve to support standard operations is a valuable dev endeavor. In this particular case, having individual views for each method helps developers implement more modular solutions.

I stumbled upon this repo randomly and apologize if this a trivial matter or outside of scope. Looks promising, but this part of the README put me off a bit. If minik is able to integrate using AWS' SAM, can I not test my functions locally using Docker with lambci/lambda and sam local? If so, the README should be updated accordingly.

This request is for functionality that would run a method called within a new lambda instance. This can be done currently using lambda:InvokeFunction, but it would be nice to have some "syntactic sugar" to simplify the implementation.

Zappa has implemented it, and it's something that we make sure of in our project. Having it in Minik will make it easier for us to migrate away from Zappa. https://github.com/Miserlou/Zappa#asynchronous-task-execution

WHAT

Minik uses a set of fixtures that create the a simple version of the event a lambda function will receive when invoked by an AWS service. Minik uses these fixtures for testing purposes and they are a convenient way of building the complex objects sent by the API Gateway and the ALB.

• Distribute a version of these utility functions

I sent a DELETE request with the following URL an API using Minik:

http://registry-private.ops.dev.sgd.eab.com/organizations?organization_name=Baijiu+University+andy

Looking at logs that printed outapp.request.query_params['organization_name'] I get the following:

{'organization_name': 'Baijiu+University+andy'}

This leads me to believe that query_params are not being fully decoded. If I traced the code right it looks like the method doing the decoding is minik.builders.url_decode_params and it's only using url_decode_params which will convert %20 to spaces in a URL path. In addition to that I think you also need to use urllib.parse.unquote_plus to convert plus signs to spaces.

This is untested but I think the following will work:

def url_decode_params(query_params):
    """
    Decode the key value pairs of a set of parameters.
    """
    def decode_string(key_or_value):
        return urllib.parse.unquote(urllib.parse.unquote_plus(key_or_value))

    return {
        decode_string(key): decode_string(value)
        for key, value in query_params.items()
    }

A best practice when developing APIs is versioning, and it's part of the EAB API standards. Minik currently doesn't have a standard way of implementing API versioning. This request is to add it.

One way of doing this is how Hug has implemented it, which appears to be fairly easy to manage within the API method decorators:
@hug.get('/echo', versions=range(2, 5))
def echo(text):
return 'Echo: {text}'.format(**locals())

https://www.hug.rest/

What

I want to have the ability to validate a request before my view gets executed. The validator's only responsibility would be that of making sure that the payload of the request is valid. Also, the validator should have access to the route, headers...

@app.route('/users', methods=['POST'], validator=UserValidator())
def create_users():
    created_users = get_created_users(app.request.json_body)
    return [u.to_dict() for u in created_users]

Minik version (0.5.1) is not correctly handling requests when the queryParameters is None. Before, if no parameter was passed in, minik would set an empty dictionary to the request.query_params, this is no longer the case.

{'error_message':
"'NoneType' object has no attribute 'get'", 'trace':
'Traceback (most recent call last):\n File "/var/task/minik/core.py", 
line 139, in error_handling\n yield\n File "/var/task/minik/core.py", 
line 115, in __call__\n self.response.body = route.evaluate(self.request)\n 
File "/var/task/minik/router.py", line 69, in evaluate\n 
return self.endpoint(**request.uri_params)\n File "/var/task/router/app.py", 
line 27, in index\n if app.request.query_params.get(\'sync\', False):\nAttributeError:
\'NoneType\' object has no attribute \'get\'\n'
}

What

I want to use minik to handle requests from an Application Load Balancer (ALB). A lambda function can be configured to be the target of an ALB and it would be nice to have a minimal web framework support the request sent by the ALB to the lambda function.

Minik should become agnostic of the service invoking the lambda function. Right now minik only supports requests that come from the API Gateway. The framework can parse those events and route a request to the correct view. However, I should be able to have the exact same web framework definition independent of the service.

• Ability to parse a request that comes from the ALB
• Ability to route an event from the ALB
• I should not have to change my code to define the request source

What

Add a debug flag as a way to get more context on what's going on behind the scenes. Add a debug flag at the app level as a way to enable this mode.

app = Minik(debug=True)

• Log internal messages out to the console (cloudwatch)
• With unhandled exception is caught during the execution of a view, instead of responding with a 500 "internal message error" include the context of the exception in the response.

Actual

When defining the following structure, I am unable to make a GET request because the second definition overrides my get. Right now the only way to troubleshoot this issue is to make a GET request on the endpoint and getting a Method is not allowed error.

@app.route("/events/{zip_code}", methods=['GET'])
def get_events(zip_code):

    if zip_code == '20902':
        return {'events': ['MD Gran fondo', 'Old Busthead']}
    return {'events': ['other events']}


@app.route("/events/{zip_code}", methods=['POST'])
def post_event(zip_code):

    event_name = app.current_request.json_body.get('name')
    # Save this event somewhere
    return {'id': 100}

Expected

I should either be able to define this sort of endpoint in which the route is identical for both the GET and the POST but the views are different, or I should throw an exception during runtime.

Django lets you add validation to path parameters in its URL config.

# Using django.urls.path
'tenant/<uuid:tenant_uuid>/'

# Using django.urls.re_path
r'tenant/(?P<tenant_uuid>[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{8})/'

I'd like something similar in Minik.

What

While writing the documentation example I noticed that there are a few fields from the context, that minik is not currently exposing to the caller. For instance the StageVariables, these are values configured at the API Gateway level, that will never get passed in to the view.

This is the documentation of the fields in the event: https://docs.aws.amazon.com/lambda/latest/dg/eventsources.html#eventsources-api-gateway-request

How

Either keep a copy of the original event in the request so that a developer can write something like:

dev view():
    app.request.event.get('stageVariables')

Or implement a refactoring of the request object to load properties on demand.

Why

If a developer has a set of stage variables set and he's not able to access them, using minik would be frustrating.

What

Handle the case in which the request is gzipped. Make sure we can gzip and gunzip the payload of a given request when the request sends data zipped. This feature should be implemented after the changes to support multiple content-types.

How

While processing a request, unzip the payload on demand, and gzip the response back.

Why

For optimization purposes, being able to support gzipped communication is important.

What

I want to have the ability to define an object schema using a library like marshmallow.

The schema definition should validate as well as automatically convert a given payload to an instance of the object as defined by the schema. Proposed syntax:

class ArtistSchema(Schema):
    name = fields.Str()

@app.route('/artist', methods=['POST'], schema=ArtistSchema())
def create_artist():
    artist = app.request.object
    save_artist(artist)
    return {"name": artist.name}

WHAT
Enhance minik as a way to streamline the definition of a route for a single http method. Right now minik accepts a kwarg as a way to limit the scope of a view for a given route.

Current implementation:

@app.route('/benchmark/{label}', methods=['DELETE'])
def delete_me(label: str):
    return {'complete': True}

Enhanced syntax:

@app.delete('/benchmark/{label}')
def delete_me(label: str):
    return {'complete': True}

This syntax should apply to the other http methods as:

@app.get('/benchmark/{label}')
@app.post('/benchmark/{label}')
@app.put('/benchmark/{label}')

What

As a developer when writing a serverless API using minik, I cannot specify the content type for my request. The framework is currently assuming that every request is json and it's responding in json format.
Make the framework more flexible when it comes to the content type specified in the headers.

How

Right now minik is actually ignoring the value of the content type in the headers. By default the framework assumed a json communication. As part of the core, make the request response more flexible to support multiple content types.

Why

At this point the framework is too restrictive when it comes to support of content types. Being able to support basic functionality is fundamental for the growth of the framework.