-
-
Notifications
You must be signed in to change notification settings - Fork 339
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
Added a distribution tab to visualize request time distributions over time #281
base: master
Are you sure you want to change the base?
Changes from 9 commits
1475408
7dddd0e
15bdb19
8584a5f
65de365
2a72e12
cd8fef3
b7c903d
3bfb00e
6905851
b2a73d8
1ae7775
c9719ae
4465479
2d9ac63
4fa0574
ee6d6cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,3 +11,4 @@ Django>=1.11 | |
freezegun>=0.3 | ||
factory-boy>=2.8.1 | ||
gprof2dot>=2016.10.13,<2017.09.19 | ||
dealer>=2.0.5 | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,5 +57,6 @@ def read_md(f): | |
'autopep8', | ||
'pytz', | ||
'gprof2dot<2017.09.19', | ||
'dealer', | ||
] | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,15 +2,37 @@ | |
|
||
from django.utils import six | ||
|
||
from dealer.auto import auto | ||
|
||
from silk.singleton import Singleton | ||
|
||
|
||
|
||
def default_permissions(user): | ||
if user: | ||
return user.is_staff | ||
return False | ||
|
||
|
||
def default_revision(): | ||
revision = auto.revision | ||
try: | ||
# This is a pretty flaky way of getting revisions in a semi chronological | ||
# order (of course two commits from the same second cannot be correctly | ||
# ordered) for a couple of reasons: | ||
# | ||
# - it only works for git | ||
# - it's using the protected _repo attribute from dealer | ||
# - the timestamp becomes visible in the revision | ||
# | ||
# However it is better than order commit hashes alphabetically | ||
timestamp = auto._repo.git('show -s --format=%at ' + revision) | ||
revision = ' # '.join([timestamp.decode(), revision]) | ||
except: | ||
pass | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this make more sense to be And does the rest of the app work correctly with an empty string or There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @albertyw I removed the dealer dependency now - I think it's simpler just to let users to set the revision themselves using whichever method they prefer. This is what we have ended up doing when running this branch in production. |
||
return revision | ||
|
||
|
||
class SilkyConfig(six.with_metaclass(Singleton, object)): | ||
defaults = { | ||
'SILKY_DYNAMIC_PROFILING': [], | ||
|
@@ -28,7 +50,10 @@ class SilkyConfig(six.with_metaclass(Singleton, object)): | |
'SILKY_INTERCEPT_PERCENT': 100, | ||
'SILKY_INTERCEPT_FUNC': None, | ||
'SILKY_PYTHON_PROFILER': False, | ||
'SILKY_STORAGE_CLASS': 'silk.storage.ProfilerResultStorage' | ||
'SILKY_REVISION': default_revision(), | ||
'SILKY_POST_PROCESS_REQUEST': lambda x: None, | ||
'SILKY_STORAGE_CLASS': 'silk.storage.ProfilerResultStorage', | ||
'SILKY_DISTRIBUTION_TAB': False, | ||
} | ||
|
||
def _setup(self): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
# -*- coding: utf-8 -*- | ||
# Generated by Django 1.11.3 on 2017-12-08 12:52 | ||
from __future__ import unicode_literals | ||
|
||
from django.db import migrations, models | ||
import silk.storage | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
('silk', '0006_fix_request_prof_file_blank'), | ||
] | ||
|
||
operations = [ | ||
migrations.AddField( | ||
model_name='request', | ||
name='revision', | ||
field=models.TextField(blank=True, default=''), | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -206,16 +206,21 @@ def __init__(self, value): | |
super(MethodFilter, self).__init__(value, method=value) | ||
|
||
|
||
def filters_from_request(request): | ||
class RevisionFilter(BaseFilter): | ||
def __init__(self, value): | ||
super(RevisionFilter, self).__init__(value, revision=value) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need of expliclit RevisionFilter, self) in super() |
||
|
||
|
||
def filters_from_query_dict(query_dict): | ||
raw_filters = {} | ||
for key in request.POST: | ||
for key in query_dict: | ||
splt = key.split('-') | ||
if splt[0].startswith('filter'): | ||
ident = splt[1] | ||
typ = splt[2] | ||
if ident not in raw_filters: | ||
raw_filters[ident] = {} | ||
raw_filters[ident][typ] = request.POST[key] | ||
raw_filters[ident][typ] = query_dict[key] | ||
filters = {} | ||
for ident, raw_filter in raw_filters.items(): | ||
value = raw_filter.get('value', '') | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking out dealer, it seems dealer may be out of date. Have you tried testing with django >= 1.10 (klen/dealer#24)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will take a look for an alternative to
dealer
. I think the revision fuctionality is not that useful in production, at least we don't use it, we setSILKY_REVISION
to a version number insettings.py
. But it is quite useful for benchmarking locally, for example for comparing a number of commits. If I can't find a better alternative I guess another option is to just remove this dependency and let users set to the revision number using whichever method they prefer (probably nice to have an example in the readme)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will check out the failing tests.