EXPERIMENTAL FEATURE: API Overview Page #72
Conversation
|
Looks like we are using more and more of features of flask. I guess it is wise to start using flask rather than wasting our efforts reinventing each of those features. |
|
But do we need a full fledged web framework? We might just be using the templating feature of Flask which Jinja is very well providing us with. |
|
Already re-invented context globals, Jinja now and routing tomorrow. Also I don't think your usage of jinja template is very effective. Creating FileSystemLoader everytime you need a template is expensive. Ideally, the template should be compiled on the first use and reused for subsequent calls. Flask takes care of it automatically. |
|
Okay. The loader can be instantiated once. So what do you suggest? Modifying most things in firefly to use Flask? As routing is most probably the next feature that will be implemented. |
firefly/app.py
Outdated
| @@ -22,6 +23,10 @@ | |||
| ctx = threading.local() | |||
| ctx.request = None | |||
|
|
|||
| loader = FileSystemLoader(searchpath=get_template_path()) | |||
| env = Environment(loader=loader) | |||
| template = env.get_template('index.html') | |||
There was a problem hiding this comment.
You can use PackageLoader instead computing the template path. See http://jinja.pocoo.org/docs/2.10/api/#basics.
There was a problem hiding this comment.
Ah! I didnt know about this. This is a robust abstraction.
firefly/app.py
Outdated
| functions = [ | ||
| {'name': name, 'path': spec['path'], 'doc': spec['doc'], 'parameters': spec['parameters']} | ||
| for name, spec in self.generate_function_list().items() | ||
| ] |
There was a problem hiding this comment.
Wouldn't the following line simple enough?
You are calling the function generate_function_list and that is returning a dictionary? Sounds confusing.
There was a problem hiding this comment.
generate_function_list was already there before. It's good that this was pointed out.
| response.status = 200 | ||
| response.text = html | ||
| return response | ||
|
|
There was a problem hiding this comment.
Wouldn't render_docs a better name for this function?
| <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.12.4/jquery.min.js"></script> | ||
| <script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/js/bootstrap.min.js"></script> | ||
| </body> | ||
| </html> |
There was a problem hiding this comment.
Please split this into layout.html and index.html.
There was a problem hiding this comment.
Is that really needed? For future use cases?
firefly/utils.py
Outdated
| firefly_module = __import__('firefly') | ||
| file = firefly_module.__file__ | ||
| return '/'.join(file.split('/')[:-1]) + '/templates' | ||
|
|
There was a problem hiding this comment.
See my comments in app.py about using PackageLoader.
Currently, the firefly application serves a
jsondocs page at '/`.The motive is to beautify that data in the form of HTML pages and add features such as trying out the API endpoint from the page itself.
TODO
[x] Add a basic for POC
[x] Deciding on a path to host the the page. Currently, it is at
/index.htmlto prevent any conflicts\docsis finalized--- Milestone 1 ---
[ ] Add support to firefly to accept an API name parameter through firefly.yml
[ ] Finalizing an API template according to optimal UX
--- Milestone 2 ---
[ ] Add testing features
--- Milestone 3 ---
More feature suggestions are welcome.
Attached
Preliminary UI for page
