disqus / gargoyle Goto Github PK
View Code? Open in Web Editor NEWFeature switches in Django
Home Page: http://engineering.disqus.com
License: Apache License 2.0
Feature switches in Django
Home Page: http://engineering.disqus.com
License: Apache License 2.0
My first time trying out switch creating, I used a name that was more than 32 characters and nothing happened via the interface (it just hung). I saw there was a 500 via runserver, and it was a bit confusing. Looks like the issue is that the limit is 32 characters for both and there's no validation response.
Example in readme shows:
import gargoyle
Which leads to an import error
Should read:
from gargoyle import gargoyle
For some reason, the line
return self.model._meta.verbose_name.title()
returns a Lazy object returned unexpected type
for me.
https://github.com/disqus/gargoyle/blob/master/gargoyle/conditions.py#L318
Changing it to the following is a workaround.
return self.model._meta.verbose_name_raw.title()
Here is the output of self and self.model from the console.
ipdb> self
<UserConditionSet: User>
ipdb> self.model
<class 'django.contrib.auth.models.User'>
I am not sure if this is related to gargoyle, or my set up, but I can say that I followed the steps for gargoyle and nexus integration, that is, I only have gargoyle in my installed apps, but use nexus for autodiscovery.
(I don't need from future import absolute_import
from gargoyle.conditions import ConditionSet
in that case, do I?)
Functionality (that depends on jQuery) to control Switches seems to be broken for any switch that have "dot" character in its name (e.g. "blog.comments.globally_disabled"). Can anyone reproduce? The bug seem to have appeared after commit d9d8277. Anyone seeing a solution that would be backwards compatible for anyone actually using the element ids?
Generally not that problematic ofcourse, but because there are ._.....html
files in the package my compile_templates
function (which compiles all .html
files to .pyc
files) breaks completely.
At present I can't find a way to customise the media url for the gargoyle media.
Nexus allows you to specify NEXUS_MEDIA_PREFIX in your settings file. However, gargoyle does not pay any attention to this or offers no alternate solution.
Presently the index.html template uses {% url gargoyle:media ... %}, this should be changed for scenarios where the developer does not wish to use the default configuration.
I'm trying to figure out why the conditions in the switches are OR:ed rather than AND:ed.
How would I go about if I wanted:
Sorry for such a stupid question.
Thank you
Is it possible to create switches based on properties in a custom user_profile model? For example, our profile model has a "Division" property: think "marketing", "sales", etc. And we'd like to enable features for specific divisions.
If this functionality isn't available, could you provide guidance on implementing such a feature? I'd be willing to contribute, if I can.
Thanks.
When setting WSGIDaemonProcess process=[>1], the updated memcache that refreshes when changing switch status is only visible with one process. For example, as far as resulting behavior, in an environment where WSGIDaemonProcess process=2, every other page request will appear to have the correct updated switch status (process used goes back-and-forth between the process with correctly refreshed memcached switch status and the other process oblivious to the switch update). Restarting apache refreshes memcache for all processes, however this is not a sustainable approach given the intended usage of switches.
I noticed a setting "GARGOYLE_CACHE_NAME" in the switch manager. Same faulty behavior exists when designating a cache for this setting. No documentation seems to exist for any of this.
Gargoyle should be able to handle values other than True/False for switches. This allows you to avoid duplication of "settings" when you want to say "this is either on, or off, and when its on the value should be FOO".
An example API may look like:
sqt = int(gargoyle.get_value('log_slow_query_threshold'))
if sqt:
logging.info('slow query over threshold of %d!', sqt)
The default values for get_value would be True
if enabled, False
if not.
In the UI upon add/edit switch you'd be able to customize these values. Considerations are if they should only be strings. Otherwise we could allow JSON values but that means you'd be required to quote strings.
We'd also add the ability to set per-condition specific values. So if condition matches foo, the return value is 100 instead of 10.
Our gargoyle install very sporadically tosses a type error modeldict/base.py:26 where a simple return self._local_cache[key]
fails because _local_cache
is None.
I suspect this is due to a race condition as we have gargoyle configured using a locmem cache, which should be reliable, and so far it appears that all of these have occurred early after a new server process startup.
Either I've understood the documentation wrong, or there's a problem when trying to test if the user is not anonymous. A switch with a "not Anonymous" condition seems to never be active.
Here's how to reproduce:
mkdir /tmp/gargoyletest
cd /tmp/gargoyletest
virtualenv --no-site-packages --distribute .
. bin/activate
git clone https://github.com/disqus/gargoyle.git
cd gargoyle
python setup.py develop
python setup.py test
rm -rf *.egg
pip install Django==1.4.1 South==0.7.6
cd /tmp/gargoyletest/gargoyle/example_project
./manage.py runserver
In a browser:
In the terminal, type Ctrl-C and continue:
cat >>urls.py <<EOF
from django.http import HttpResponse
from django.template import Context, Template
urlpatterns += patterns('',
url(r'test/$',
lambda request: HttpResponse(
Template('''
{% load gargoyle_tags %}
<pre>
{% ifswitch not-anonymous %}
Is not anonymous
{% else %}
Is anonymous
{% endifswitch %}
</pre>
''').render(Context())
)
)
)
EOF
./manage.py runserver
Back in the browser:
Renders: "Is anonymous" <----------- WHY?
Renders: "Is anonymous" (as expected).
I also wrote a test for this. Is it written correctly? Run my test by hitting Ctrl-C in the terminal and pasting the following:
cd /tmp/gargoyletest/gargoyle
patch -p0 <<EOF
diff --git tests/tests.py tests/tests.py
index 1d6b44a..1bdb9e4 100644
--- tests/tests.py
+++ tests/tests.py
@@ -425,6 +425,18 @@ class APITest(TestCase):
self.assertTrue(self.gargoyle.is_active('test', user))
+ switch.clear_conditions(
+ condition_set=condition_set,
+ )
+
+ switch.add_condition(
+ condition_set=condition_set,
+ field_name='is_anonymous',
+ condition='0',
+ )
+
+ self.assertFalse(self.gargoyle.is_active('test', user))
+
switch.add_condition(
condition_set=condition_set,
field_name='percent',
EOF
python setup.py test
The test we just added fails <------- WHY?
Update: now that Gargoyle supports Django 1.4, removed the 1.3 hack and installed 1.4.1 (and South 0.7.6) instead. The symptom is still same as before.
My workplace is currently on django 1.3.7 and using gargoyle. When building a dev environment today, gargoyle installed the latest of django-jsonfield, 0.9.4, which assumes it can import django.utils.six
, (but which only appears to have been added to django 1.5). As such, the developer got an ImportError.
It seems as though gargoyle intends to support django 1.3, but django-jsonfield 0.9.4 does not. I don't know if django-jsonfield intends to handle this, but perhaps gargoyle would like to, (e.g. by setting an upper limit to its requirement: django-jsonfield>=0.8.0,<0.9.4
).
Thanks.
I'd like to change to relative imports as I want to run gargoyle inside my apps/
dir.
Let me know what you think about that.
It's highly possible that I'm just using gargoyle wrong, but I'm experiencing surprising behavior.
In one of my project's apps, I've created a gargoyle.py
file:
from __future__ import absolute_import
from gargoyle import gargoyle
gargoyle['foo'].add_condition(
condition_set='gargoyle.builtins.UserConditionSet(auth.user)',
field_name='percent',
condition='0-50'
)
My understanding is that this will make the foo
switch active for 50% of users. This seems to work. The value in the database after I run my local server is:
{
"auth.user": {
"percent": [
[
"i",
"0-50"
]
]
}
}
But now, if I restart my server, the condition is duplicated:
{
"auth.user": {
"percent": [
[
"i",
"0-50"
],
[
"i",
"0-50"
]
]
}
}
I've traced this to Switch.add_condition
:
if condition not in self.value[namespace][field_name]:
self.value[namespace][field_name].append((exclude and EXCLUDE or INCLUDE, condition))
Here it's checking if condition
which is '0-50'
is "in" a list of tuples, which will always return false. Obviously this is solvable by adding commit=False
to Switch.add_condition
. However, it makes me worry that I'm missing something fundamental.
Fix available in akaihola/gargoyle@511b7b8
$ python setup.py test
running test
running egg_info
writing requirements to gargoyle.egg-info/requires.txt
writing gargoyle.egg-info/PKG-INFO
writing top-level names to gargoyle.egg-info/top_level.txt
writing dependency_links to gargoyle.egg-info/dependency_links.txt
reading manifest file 'gargoyle.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
warning: no files found matching 'README'
warning: no previously-included files matching '*~' found anywhere in distribution
writing manifest file 'gargoyle.egg-info/SOURCES.txt'
running build_ext
Traceback (most recent call last):
File "setup.py", line 36, in <module>
'Topic :: Software Development'
File "lib/python2.7/distutils/core.py", line 152, in setup
dist.run_commands()
File "lib/python2.7/distutils/dist.py", line 953, in run_commands
self.run_command(cmd)
File "lib/python2.7/distutils/dist.py", line 972, in run_command
cmd_obj.run()
File "setuptools/command/test.py", line 137, in run
self.with_project_on_sys_path(self.run_tests)
File "setuptools/command/test.py", line 117, in with_project_on_sys_path
func()
File "setuptools/command/test.py", line 146, in run_tests
testLoader = loader_class()
File "lib/python2.7/unittest/main.py", line 94, in __init__
self.parseArgs(argv)
File "lib/python2.7/unittest/main.py", line 149, in parseArgs
self.createTests()
File "lib/python2.7/unittest/main.py", line 158, in createTests
self.module)
File "lib/python2.7/unittest/loader.py", line 128, in loadTestsFromNames
suites = [self.loadTestsFromName(name, module) for name in names]
File "lib/python2.7/unittest/loader.py", line 91, in loadTestsFromName
module = __import__('.'.join(parts_copy))
File "gargoyle/runtests.py", line 55, in <module>
from django_nose import NoseTestSuiteRunner
File "django_nose/__init__.py", line 4, in <module>
from django_nose.runner import *
File "django_nose/runner.py", line 18, in <module>
from django.core.management.commands.loaddata import Command
File "django/core/management/commands/loaddata.py", line 13, in <module>
from django.core import serializers
File "django/core/serializers/__init__.py", line 21, in <module>
from django.core.serializers.base import SerializerDoesNotExist
File "django/core/serializers/base.py", line 7, in <module>
from django.db import models
File "django/db/__init__.py", line 40, in <module>
backend = load_backend(connection.settings_dict['ENGINE'])
File "django/db/__init__.py", line 34, in __getattr__
return getattr(connections[DEFAULT_DB_ALIAS], item)
File "django/db/utils.py", line 92, in __getitem__
backend = load_backend(db['ENGINE'])
File "django/db/utils.py", line 51, in load_backend
raise ImproperlyConfigured(error_msg)
django.core.exceptions.ImproperlyConfigured: 'sqlite3' isn't an available database backend.
Try using django.db.backends.sqlite3 instead.
Error was: No module named base
I know that disqus is not using >1.4, but do you have user reports as to whether the library is working properly in 1.6? In my limited testing locally, I haven't run into any problems yet.
Thank you.
Now with "switch_is_active" is possible to put a single view behind a switch, can I put an include behind a switch?
Something like this: ('^url', switch_is_active('name')(include('app/urls.py'))
Adding a switch condition fails in Django 1.2.5 due to the new CSRF protections.
The syntax for SwitchContextManager is with switches(my_switch_name=True):, which requires my_switch_name to be a valid Python identifier.
Hierarchical switches use ":" to separate path components - which isn't a valid character in an identifier.
I get this when I try to include gargoyle in a blank project (well, a project that includes Nexus and Sentry) and syncdb:
django.core.exceptions.ImproperlyConfigured: ImportError gargoyle: No module named jsonfield
If I try to runserver I get:
Error: No module named jsonfield
These imports in models.py show up as unresolved:
from jsonfield import JSONField
from modeldict import ModelDict
Thanks!
I'm curious as to when the next release is coming. Rather tahn me directly pinging zeeg on twitter or freenode, perhaps there could just be a rough estimate and future plan in the project README or something?
Possibly within a modal, add in support for testing a switch against given conditions
Something along the lines of what Disqus has in its legacy system:
I'm using gargoyle with nexus. It works perfectly on my development machine. On my production environment (mod-wsgi daemon mode) the condition sets only seem to be getting registered in some of the processes (at least that is what I think is happening - if I open the gargoyle admin page for a switch and keep reloading the page the conditions only show up sometimes).
I have them both in my INSTALLED APPS
INSTALLED_APPS = (
...,
'nexus',
'gargoyle',
...,
)
and in my urls.py:
import nexus
nexus.autodiscover()
Any ideas on what the problem could be?
The documentation for {% ifswitch %}
gives two examples. The first one doesn't quote the switch name:
{% ifswitch switch_name %}
switch_name is active!
{% else %}
switch_name is not active :(
{% endifswitch %}
whereas the second one does:
{% ifswitch "my switch name" user %}
"my switch name" is active!
{% endifswitch %}
Based on a quick test and a brief look at the code for the template tag it seems that a quoted argument is not resolved against the template context, so the quotes are included in the switch name. A quoted switch name in the template tag doesn't match an unquoted switch name created in Nexus.
Strangely enough, a switch with a quoted name can't be created if a similarly named unquoted switch already exists.
User's real ip is present in HTTP_X_FORWARDED_FOR
, if they are behind a proxy or a reverse proxy.
When working with a growing number of switches eventually there will be a moment when you'd like to organise things a bit more.
Currently, one is presented with a list of switches in the admin interface. If you don't follow a nifty naming scheme it's hard to maintain overview and quickly find what you're looking for.
What if switches could be organised by something like app/feature/tags? A switch could have an optional field like feature
. Question is how to make this conveniently browsable via /admin.
0-50% should mean lower 50%.
Although, because IPAddressConditionSet.get_field_value
returns the output of _ip_to_int
without taking care of upper bounds, it doesn't work as expected.
When USE_TZ = True is specified in a settings file, RuntimeWarnings are thrown in the migrations. This is because the default times in the parameters for the field instantiation are not timezone aware. Patch forthcoming.
I've been working on support for this as we need it in my company.
Can I get permission to create PRs?
Tried installing gargoyle 0.6.1 with nexus 0.2.3 and none of the ajax links work for managing switches, due to the standard csrf errors. 403 forbidden CSRF verification failed. Request aborted.
The recent nexus commits to use ajaxSetup are being called correctly, but something (I've no idea what) is not working with gargoyles provided templates and staticfiles.
Getting the following DeprecationWarning notices when I run runserver on my Django 1.3 app with the latest version of Gargoyle installed (tried both through pip install and straight from github):
/usr/local/lib/python2.6/dist-packages/django/db/models/fields/subclassing.py:80: DeprecationWarning: A Field class whose get_db_prep_save method hasn't been updated to take a `connection` argument. new_class = super(SubfieldBase, cls).__new__(cls, name, bases, attrs) /usr/local/lib/python2.6/dist-packages/django/db/models/fields/subclassing.py:80: DeprecationWarning: A Field class whose get_db_prep_value method hasn't been updated to take `connection` and `prepared` arguments. new_class = super(SubfieldBase, cls).__new__(cls, name, bases, attrs)
Hi,
I see from docs 1.5 is not supported; any idea when support will be added for it and what breaks atm ?
Hi using gargoyle.is_active we've found in our sentry this exception:
Stacktrace (le chiamate più recenti alla fine):
File "django/core/handlers/base.py", line 164, in get_response
response = response.render()
File "django/template/response.py", line 158, in render
self.content = self.rendered_content
File "django/template/response.py", line 135, in rendered_content
content = template.render(context, self._request)
File "django_jinja/backend.py", line 64, in render
return self.template.render(context)
File "jinja2/environment.py", line 989, in render
return self.environment.handle_exception(exc_info, True)
File "jinja2/environment.py", line 754, in handle_exception
reraise(exc_type, exc_value, tb)
File "./yellow/our_template.html", line 83, in block "main_content"
{{ render_company(company_name, switch_is_active("ourswith", request.user)) }}
File "yellow/templatetags/tags.py", line 137, in switch_is_active
return gargoyle.is_active(switch_slug, *instances)
File "gargoyle/manager.py", line 58, in is_active
switch = self[key]
File "gargoyle/manager.py", line 34, in __getitem__
return SwitchProxy(self, super(SwitchManager, self).__getitem__(key))
File "modeldict/base.py", line 23, in __getitem__
self._populate()
File "modeldict/base.py", line 168, in _populate
elif self.local_cache_has_expired():
File "modeldict/base.py", line 108, in local_cache_has_expired
recheck_at = self._last_checked_for_remote_changes + self.timeout
Seems to be a concurrency issue in the function local_cache_has_expired
105. if not self._last_checked_for_remote_changes:
106. return True # Never checked before
107.
108. recheck_at = self._last_checked_for_remote_changes + self.timeout
109. return time.time() > recheck_at
We use:
django 1.8.4
gargoyle 0.11.0
If we've never used gargoyle or gutter on any projects before and would like to use one on a new project, should we definitely use gutter? Or are there any features or functionality or any advantages to using gargoyle over gutter?
This is a pretty serious bug introduced by #53.
Since this commit, the order of the conditions is now important, and since this library is backed by modeldict
, that order can not be guaranteed. Say you have multiple conditions over multiple fields:
127.0.0.1
[email protected]
If the conditions come out in the order above, then the condition set is always true. If they come out in the opposite order, the condition set is only true if the IP address is 127.0.0.1
.
What we're seeing is that the conditions are sometimes respected correctly (resulting in the expected behaviour), but the vast majority of the time the switch is just enabled. It's almost impossible to reproduce by running the code in a shell, because the order of a dictionary changes based on how the memory is laid out, and that's unique for each process.
Furthermore, I don't think the "default" should ever be that a switch is on. If a switch is in selective mode and has a single exclude condition, then that switch should not be enabled for anyone. You should have to selectively opt-in groups users to the switch, then opt-out individuals if necessary.
I don't expect there's any chance of the default behaviour being changed again, so I'll just update our fork to revert this change, but I wanted to raise this in case anyone comes across this crazy bug in the future!
is there a migration path between these two, or is it not straightforward?
In the case where a view has a gargoyle switch decorator, it's not clear what happens if the switch is "off" for the user. Will it return a 404? A one-sentence explanation in the readme should be sufficient.
Similar to Issue #19, we're running apache with mod_wsgi, Django 1.4 and Gargoyle 0.10.3. The difference with 19 is that we always see the condition in nexus, but on the actual site it's wildly inconsistent or slow to pick up the condition change. Best guess is that it's picking up by thread. If we restart Apache everything looks good. Using the Django development server also always works.
If it's any help/makes any difference, we're using gargoyle.is_active to check the conditions.
Nexus' nexus.js includes a beforeSend function for the Jquery global setup to pass the CSRF protection token into the headers before any XHR call. When the XHR request is sent from gargoyle.js say to add a new switch its adding another copy of the token into the header. This causes Django 1.3 to reject the request with a invalid token.
gargoyle.js should check if the header has already been defined, if so skip setting the header again. Then again, as this segment of code would not be called unless Nexus is installed maybe it should be removed.
I ran into a compatibility issue with Django 1.8, but after poking around a bit, I notice the head of this project has fixed it. The last release of gargoyle is 0.11.0 though, which doesn't include that fix. Any chance of a new release? I'd like to be able to manage this dependency with pip properly.
Right now it just silently moves on, which makes it hard to figure out why all your extensions are not registered.
gargoyle uses modeldict for retrieving Switches. modeldict has two levels of caching: an in-memory cache based on key and the standard Django cache.
If a test sets a switch by specifying a fixture that stores the switch values directly in the database, then later tests that don't use the fixture might still get the changed value of the switch. If the Django cache is, say, an external memcache, then later test runs might also retrieve the cached value.
As far as I can tell, gargoyle has no way of clearing the caches nor of asking it to not use these caches in tests.
https://code.launchpad.net/~jml/canonical-identity-provider/reliable-tests/+merge/136932 has some more details about the problem.
This is just what I've figured out from exploring. I'm no Django expert. If this isn't actually a bug, I would be glad to learn how to get proper test isolation. Am happy to answer questions.
Thanks,
jml
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.