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.

Sign up for GitHub

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

EXPERIMENTAL FEATURE: API Overview Page #72

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

EXPERIMENTAL FEATURE: API Overview Page #72

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f30c5eb
Add utility to find the templates dir path
palnabarun Dec 8, 2017
2769302
Adds functionality to render a showcase page
palnabarun Dec 8, 2017
fe2cd9a
Updates requirement.txt with Jinja2
palnabarun Dec 8, 2017
07c2bd3
Moves the FileSystemLoader so that it is instantiated onle once.
palnabarun Dec 11, 2017
c3c929e
Moves the browser path to /docs
palnabarun Dec 11, 2017
d4bf9d6
Adds the browser UI template
palnabarun Dec 11, 2017
817e3a6
Removes the instance of API name
palnabarun Dec 11, 2017
35eee6e
Updates the index template with usage instructions
palnabarun Dec 11, 2017
456b0f4
Adds support to show the host url in docs
palnabarun Dec 11, 2017
8844d34
Updates docs template with reviews
palnabarun Dec 11, 2017
6aacc8d
Uses jinja2.PackageLoader instead of FileSystemLoader along with comp…
palnabarun Dec 11, 2017
fa5dfcb
Refactors generate_showcase to render_docs
palnabarun Dec 11, 2017
dfeab61
Removes non-existing import
palnabarun Dec 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions firefly/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import cgi
from webob import Request, Response
from webob.exc import HTTPNotFound
from jinja2 import FileSystemLoader, Environment
import json
import logging
from .validator import validate_args, ValidationError
from .utils import json_encode, is_file, FileIter
from .utils import json_encode, is_file, FileIter, get_template_path
from .version import __version__
import threading

Expand All @@ -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')
Copy link
Member

Choose a reason for hiding this comment

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

You can use PackageLoader instead computing the template path. See http://jinja.pocoo.org/docs/2.10/api/#basics.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah! I didnt know about this. This is a robust abstraction.


class Firefly(object):
def __init__(self, auth_token=None):
self.mapping = {}
Expand All @@ -47,6 +52,20 @@ def generate_index(self):
}
return help_dict

def generate_showcase(self, **kwargs):
functions = [
{'name': name, 'path': spec['path'], 'doc': spec['doc'], 'parameters': spec['parameters']}
for name, spec in self.generate_function_list().items()
]
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't the following line simple enough?

You are calling the function generate_function_list and that is returning a dictionary? Sounds confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

generate_function_list was already there before. It's good that this was pointed out.

html = template.render({
'host_url': kwargs['host_url'],
'functions': functions
})
response = Response(content_type='text/html')
response.status = 200
response.text = html
return response

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't render_docs a better name for this function?

Copy link
Contributor Author

@palnabarun palnabarun Dec 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Agreed.

def __call__(self, environ, start_response):
request = Request(environ)
response = self.process_request(request)
Expand All @@ -73,7 +92,9 @@ def process_request(self, request):
ctx.request = request

path = request.path_info
if path in self.mapping:
if path == "/docs":
return self.generate_showcase(host_url=request.environ['HTTP_HOST'])
elif path in self.mapping:
func = self.mapping[path]
response = func(request)
else:
Expand Down
78 changes: 78 additions & 0 deletions firefly/templates/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>{% if name %}{{ name }} | {% else %}{% endif %}firefly | API Browser</title>
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css">
<style type="text/css">
.bs-example{
margin: 20px;
}
</style>
</head>
<body>
<div class="container">
<br>
<h3>{% if name %}{{ name }} | {% else %}{% endif %}firefly | API Browser</h3>
{% if version %}
<p><b>Version: </b>{{ version }}</p>
{% endif %}
<div class="row">
<div class="col-md-6">
<div class="bs-firefly">
{% for function in functions %}
<div class="panel-group" id="accordion">
<div class="panel panel-default">
<div class="panel-heading">
<h4 class="panel-title">
<div class="pull-right">{{ function.path }}</div>
<a data-toggle="collapse" data-parent="#accordion" href="#collapse{{ loop.index }}">{{ function.name }}</a>
</h4>
</div>
<div id="collapse{{ loop.index }}" class="panel-collapse collapse in">
<div class="panel-body">
<p>
{% if function.doc %}
{{ function.doc }}
{% else %}
<i>No docstring provided</i>
{% endif %}
</p>
<h4>Parameters</h4>
<ul>
{% for param in function.parameters %}
<li>{{ param.name }}</li>
{% endfor %}
</ul>
</div>
</div>
</div>
</div>
{% endfor %}
</div>
</div>
<div class="col-md-6">
<p>The API can be used by using firefly-client</p>
<p>Install it using:</p>
<div class="well">
<samp>
$ pip install firefly
</samp>
</div>
<p>Usage:</p>
<div class="well">
<samp>
>>> import firefly<br/>
>>> client = firefly.Client('{{ host_url }}')<br/>
>>> client.function_name(&lt;parameters&gt;)<br/>
&lt;function_response&gt;
</samp>
</div>
</div>
</div>
<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>
Copy link
Member

Choose a reason for hiding this comment

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

Please split this into layout.html and index.html.

Copy link
Contributor Author

@palnabarun palnabarun Dec 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Is that really needed? For future use cases?

5 changes: 5 additions & 0 deletions firefly/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ def json_encode(data):
def is_file(obj):
return hasattr(obj, "read")

def get_template_path():
firefly_module = __import__('firefly')
file = firefly_module.__file__
return '/'.join(file.split('/')[:-1]) + '/templates'

Copy link
Member

Choose a reason for hiding this comment

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

See my comments in app.py about using PackageLoader.

class FileIter:
def __init__(self, fileobj, chunk_size=4096):
self.fileobj = fileobj
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ WebOb==1.7.2
requests>=2.18.1
PyYAML==3.12
funcsigs==1.0.2 ; python_version < '3'
Jinja2==2.10
3 changes: 2 additions & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ def get_version():
'gunicorn==19.7.1',
'WebOb==1.7.2',
'requests==2.18.1',
'PyYAML==3.12'
'PyYAML==3.12',
'Jinja2==2.10'
]

if PY2:
Expand Down