Skip to content
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

context parameter for @app.schedule lambdas #856

Closed
xplorld opened this issue Jun 1, 2018 · 4 comments
Closed

context parameter for @app.schedule lambdas #856

xplorld opened this issue Jun 1, 2018 · 4 comments

Comments

@xplorld
Copy link
Contributor

xplorld commented Jun 1, 2018

Currently we do not provide context to @app.schedule lambdas. It may be useful in some cases when the user wants to know who I am and maybe check remaining times.
To be backward compatible, the ScheduledEventHandler.__call__ needs to check client code's signature to support 1-param version (event) and 2-param version (event, context) client code.

By the way, the event in @app.schedule is different from event in @app.lambda_function, which may arise confusion. Maybe we can rename the parameter name to things like schedule_event or cloudwatch_event?

@kyleknap
Copy link
Contributor

kyleknap commented Jun 6, 2018

Unfortunately, I do not think we would be able to expand the signature to include a context along with an event. However, the app object does expose a lambda_context property that you could access from with in the schedule event function. So something like this:

from chalice import Chalice
from chalice.app import Rate

app = Chalice(app_name='ping')


@app.schedule(Rate(1, Rate.MINUTES))
def index(event):
    print(app.lambda_context)
    return {'hello': 'world'}

The one thing I did notice though is that the ScheduledEventHandler does not set the lambda_context property to the context object that gets passed through right now, but we could update the code to set it. Would using app.lambda_context work for you? @jamesls do you have any thoughts on this?

As to renaming event to schedule_event, that is something you should be able to control in defining your function. We just use event in the examples by convention. Not sure if it is intentional (@jamesls would know more about this) but we pass the event in as a positional argument so the name you give the argument does not matter. If we switched to be passing it in as a keyword argument that would probably cause application to break and thus something we would avoid doing. So you should be safe there.

@jamesls
Copy link
Member

jamesls commented Jun 7, 2018

do you have any thoughts on this?

The .lambda_context was needed because the args in a route() function's were controlled by the user, so we needed somewhere else to place the context.

Could we just add a context attribute to the event? Otherwise we need to have each event have a reference to the app object which I'd prefer to avoid.

@kyleknap
Copy link
Contributor

kyleknap commented Jun 8, 2018

I would be fine with that as well.

@xplorld
Copy link
Contributor Author

xplorld commented Jun 14, 2018

It would be nice to add event.context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants