Rate limiting for scheduling triggers

[Flix6x]

I completed my tech spike.

I couldn't (figure out how to) decorate only the trigger method of our FlaskView, so I worked around that by decorating the entire SensorAPI and using the cost function to only count towards the limit for the trigger method. It's not the most elegant solution, but it does look easily extensible this way.

We also discussed offline on what to apply the limit (using key_func): users or accounts. Here I decided to combine the account and the sensor id, just to let a test pass (test_trigger_and_get_schedule first schedules a battery and then schedules a charging station). But it might also make sense to apply a limit per sensor.

[nhoening]

I found two arguments against protecting a whole resource in one way.

In this solution (also using a syntactic sugar on top of Flask), they found a way to add decorators per method. Not sure if Flask-Classful has that.

Have you tried to decorate an endpoint outside of Flask-Classful's reach?

[nhoening]

We apply this here for all endpoints, but the timing we want to allow should differ per endpoint.

[Flix6x]

But given the cost function, it is only effectively applied to one endpoint. We are free to add more limiter.limit() decorators with other rate limits that affect other endpoints (via the cost function).

[nhoening]

I'm afraid that if for some reason the endpoint's identification string changes, we suddenly stop limiting it.

[Flix6x]

I can probably find a way to obtain this string in a way that directly references the class method.

SensorAPI.__name__ + ":" + SensorAPI.trigger_schedule.__name__ would be my first try.

[Flix6x]

In this solution (also using a syntactic sugar on top of Flask), they found a way to add decorators per method. Not sure if Flask-Classful has that.

I'd definitely prefer that, but did they find a way though? I don't see any comment claiming they were able to add a decorator for a distinct class method. Only for all of the methods in the class, by adding the limiter.limit() to the list of method_decorators (which is applied to all class methods). In flask-classful this list variable is called decorators instead, and it is precisely that which I'm appending the limiter.limit() to. (I didn't add it to the list in the class itself, because the app context is not known there.)

What is possible is to tell limiter.limit() to only apply itself to class methods that support a certain http method (e.g. POST). I guess we could split off the schedule triggering part from the SensorAPI class, so both new classes will only contain a single POST method. That might also be more RESTful in the end, since I've come to think about the trigger endpoint as POSTing a scheduling job.

[nhoening]

Have you tried yet to decorate an endpoint outside of Flask-Classful?

[Flix6x]

I have. That had the same problem of not having the app context available. Applying it to a whole blueprint did work, though. I had accomplished the latter in the register function of __init__.py of API v2, where the app context is available.

[nhoening]

I don't recall why the app context is needed for...

[Flix6x]

I don't recall why the app context is needed for...

It's needed to set up the Limiter (https://flask-limiter.readthedocs.io/en/stable/api.html#flask_limiter.Limiter.params.app), so all of these config variables can be read in, including the redis connection.

[nhoening]

I'm stuck here because it seems weird that such core functionality (adding the app context) would not work in the main use case (an endpoint).

It's passed to the Limiter in the main examples. Sometimes, the limiter decorator cannot access things from the app but the docs propose a solution which might work?

[Flix6x]

On this branch you'll find the rest of my tech spike where I test out limiting a non-FlaskView endpoint by decorating it directly, and also limiting an entire blueprint. The former doesn't work (the decorator seems to be ignored), the latter does work.

To test out, run pytest -k test_post_prognosis_2_0. (To test out the blueprint approach, comment out the endpoint's decorator, and uncomment my edits in v2's __init__.py.

Sometimes, the limiter decorator cannot access things from the app but the docs propose a solution which might work?

In my branch, the issue with the decorated endpoint approach seems not to be in getting the limit string from the app (I just hardcoded a string), but setting up the limiter to work with the app.

I guess you could carry on my tech spike, or we should come to a decision on how I can move forward. Otherwise, I'm stuck, too. I've made two suggestions:

• Using the cost function as a way of assigning the limit to a specific endpoint, or
• Splitting off the triggering functionality from SensorAPI.

[nhoening]

I believe I'm willing to pursue the cost function approach. I did not yet understand at which (accumulated) costs the rate limiter stops access. Is that simply 1? Or can this be some setting? And are costs accumulated per endpoint? Maybe you can point me to the place in the rate-limiter docs where I can learn this. I could not find it.

In any case, I'd still want to:

• move the cost function to a central place
• make sure the costs (per endpoint I guess) apply to an account
• maybe adjust the error message

I'm willing to work on that.

[Flix6x]

I did not yet understand at which (accumulated) costs the rate limiter stops access. Is that simply 1? Or can this be some setting? And are costs accumulated per endpoint? Maybe you can point me to the place in the rate-limiter docs where I can learn this. I could not find it.

For limiter.limit("10/second") the limit cost would be 10.

Docs here.

[nhoening]

Current list of ideas of TODOs:

• make other tests pass, as well
• base rate limiting for non-resource endpoints (e.g. ping, login)
• separate base rate limiting for resource from account-based limiting (limit only "smart" endpoints, add attribute to accounts)

[Flix6x]

Superseded by #2306, which implements rate limiting and closes #306.

The blocker that stalled this spike is gone: we could not decorate a single FlaskView method back then, which is why this branch decorated the whole SensorAPI and used a cost function returning 0 for every method except trigger_schedule. flask-classful 0.16 stacks per-method decorators fine, so #2306 puts a limit decorator directly on each trigger endpoint and drops the workaround.

The TODOs collected in the discussion here are addressed there as well: a separate base limit for all API endpoints next to the stricter trigger limit, the limit key made configurable (per asset / per account / per user), account-level overrides through a rate_limits account attribute, and a 429 response in our usual JSON error shape with a Retry-After header.

Closing in favour of #2306.