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

[WIP] Adds support for passing in a name and a Flask application to the ini… #82

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

palnabarun
Copy link
Contributor

…t of Firefly

@palnabarun palnabarun added the WIP label Oct 12, 2018
firefly/app.py Outdated
@@ -25,7 +26,7 @@
ctx.request = None

class Firefly(object):
def __init__(self, auth_token=None, allowed_origins=""):
def __init__(self, name, auth_token=None, allowed_origins="", flask_app=None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use name=None and set it to "firefly" if not specified. That way you don't have to change any of the existing code.

@anandology
Copy link
Member

Looks like you haven't added flask to requirements.txt.

@palnabarun
Copy link
Contributor Author

Yes. I didn't. I am still working on this PR.

self.mapping = {}
self.add_route('/', self.generate_index,internal=True)
self.auth_token = auth_token
self.allowed_origins = allowed_origins

self.flask_app = flask_app or Flask(name)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally it should be,
self.flask_app = flask_app if flask_app else Flask(name) as this is more readable.

The current statement also evaluates to the same result, however intent is not clear, and one has to know that Python evaluates or lazily.

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

Successfully merging this pull request may close these issues.

3 participants