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

When using application factory where should I be putting my jwt.callbacks? #14

Closed
rlam3 opened this issue Oct 25, 2016 · 7 comments
Closed

Comments

@rlam3
Copy link
Contributor

rlam3 commented Oct 25, 2016

When using application factory where should I be putting my jwt.callbacks/ How should I be organizing them and also their exception handlers? Thanks!

Because application.py is where the

from extensions import jwt_manager

create_app(): 

    app = Flask(__name__)
    jwt_manager.init_app(app)

    return app

extenstions.py

jwt_manager = JWTManager()

Where should I be putting the callback handlers for jwt to find them out?

Ex.

@jwt.expired_token_loader
def my_expired_token_callback():
    return jsonify({
        'status': 401,
        'sub_status': 101,
        'msg': 'The token has expired'
    }), 200
@vimalloc
Copy link
Owner

I think it depends on if you want to share the same jwt_manager/callbacks, or if you want each app to have their own. I haven't built any applications using a factory pattern, so I couldn't say what the best practices for this is, it will probably depend on your use case. But here is an example of how you could do it both way:

Using a different jwt_manager for each app (you could make different callbacks in here, with if statements and various config settings, if that was required)

# application.py
create_app():
    app = Flask(__name__)
    jwt = JWTManager(app)

    @jwt.expired_token_loader
    def my_expired_token_callback():
        return jsonify({
            'status': 401,
            'sub_status': 101,
            'msg': 'The token has expired'
        }), 20

Using the same JWT manager for each app

# application.py
from extensions import jwt_manager

create_app(): 
    app = Flask(__name__)
    jwt_manager.init_app(app)
    return app
# extensions.py
jwt_manager = JWTManager()

@jwt_manager.expired_token_loader
def my_expired_token_callback():
    return jsonify({
        'status': 401,
        'sub_status': 101,
        'msg': 'The token has expired'
    }), 20

You could even take it a step further and have a third (or any number of other) files that define callbacks. Functionally this would work the same as above, but depending on how your code is structured it may make more sense to organize it this way. It could look like:

# <whatever>.py
from extensions import jwt_manager

@jwt_manager.expired_token_loader
def my_expired_token_callback():
    return jsonify({
        'status': 401,
        'sub_status': 101,
        'msg': 'The token has expired'
    }), 20

Hope this helps!

@rlam3
Copy link
Contributor Author

rlam3 commented Oct 25, 2016

@vimalloc If I do it your suggested way with <jwt_callbacks.py/whatever.py> ... How will the app know when I use the application factory way? I'd still need to import it into the create_app function correct?

@vimalloc
Copy link
Owner

Yes, the create_app() function would look the same regardless if you are putting your callbacks in <whatever>.py or extensions.py.

Just to be clear, I'm not actually sure if having the callbacks in a separate <whatever>.py file would be a good idea or not, it depends on how your code is organized. It is just another option that is available to you. The key here is having your create_app function import the jwt_manager and call init_app on it, and have another place outside of the create_app function use the loader methods to register callbacks. That is assuming that you want all the applications created with the create_app function to have the same jwt callback functions.

@rlam3
Copy link
Contributor Author

rlam3 commented Oct 26, 2016

My assumption was that jwt_call_back handlers would all be the same, especially when it comes down to error handling.

Because I was thinking of how to organize it so my application.py and extensions.py doesn't appear to be too cluttered, I am thinking of using the <whatever>.py to contain the callback.
But the issue is I'd still need import the callback via <whatever>.py to register it.

Would this be the preferred way to keep code modular and clean? Or would you recommend that writing the callbacks inside extension is enough?

Also, this would probably be a good time to also discuss logging feature for error handling of the app. Since some callbacks lead to 404, or 400 bad requests. Would you consider in adding a logging mechanism for errors? Or would this be out of your scope?

Thanks!

@vimalloc
Copy link
Owner

Personally I think in most cases having callbacks inside extensions.py would be fine, but that really comes down to a choice in your application, so I don't want to say anything too concrete. Really, I think either way would work just fine.

As per logging, I think that is something that we could put into this code. I think it may make most sense to put some sane default logging in, and if you wanted anything more specific to override the callback functions (via the loader methods) and put your own logging statements in there. What kind of things would you specifically like to see logged?

I'm going to be pretty busy for the next week or so, so it may be a bit before I can properly look at these. If you need logging for now, I would recommend just overriding the various callback methods (via the loader functions) and add your logging in there.

Cheers!

@rlam3
Copy link
Contributor Author

rlam3 commented Oct 28, 2016

I'm looking errorr handlers to display warnings and errors with invalid api usage. It is useful to slap a pdb statement into the error handler and go up the stack to see where errors are.

Definitely an error_handler_callback sort of method would do the trick. Or maybe even setup logging to 3rd party apps like sentry, rollbar... etc..

@vimalloc
Copy link
Owner

vimalloc commented Nov 9, 2016

Moving the logging discussion into #17 and closing this one

@vimalloc vimalloc closed this as completed Nov 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants