Merge lp:~fgallina/conn-check/configs-dont-mess-with-caches into lp:~ubuntuone-hackers/conn-check/configs

Proposed by Fabián Ezequiel Gallina
Status: Merged
Approved by: Fabián Ezequiel Gallina
Approved revision: 31
Merged at revision: 31
Proposed branch: lp:~fgallina/conn-check/configs-dont-mess-with-caches
Merge into: lp:~ubuntuone-hackers/conn-check/configs
Diff against target: 28 lines (+10/-1)
1 file modified
conn_check_configs/django.py (+10/-1)
To merge this branch: bzr merge lp:~fgallina/conn-check/configs-dont-mess-with-caches
Reviewer Review Type Date Requested Status
Natalia Bidart Approve
Review via email: mp+324680@code.launchpad.net

Commit message

Properly handle single value CACHES locations

The `location` key in CACHES can either be a single value string or a list. The code already attempted to wrap the value if it was not an iterable, but since strings are iterables in python the logic was wrong.

To post a comment you must log in.
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

15:10 < fgallina> nessita: yeah, isinstance against `types.StringTypes` and `basestring` should be equivalent, but it gave me also a quick win for making it py3 compatible.
15:10 < fgallina>| hackity hack
15:11 < nessita> fgallina, and this code has no tests? :-
15:11 < nessita> wesleeeeeeeey
15:11 < fgallina> nessita: I feel most sad there are no tests for this code whatsoever and I felt compelled to add some but I didn't want to waste much time on this fix.
15:12 < fgallina> nessita: there's an empty tests.py ;_;
15:12 < nessita> ack

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'conn_check_configs/django.py'
2--- conn_check_configs/django.py 2015-12-07 03:43:57 +0000
3+++ conn_check_configs/django.py 2017-05-26 14:57:39 +0000
4@@ -6,6 +6,7 @@
5 import collections
6 import os
7 import sys
8+import types
9 import yaml
10
11
12@@ -130,7 +131,15 @@
13 'django.core.cache.backends.memcached.MemcachedCache'):
14
15 locations = cache['LOCATION']
16- if not isinstance(locations, collections.Iterable):
17+ # XXX: Hackish
18+ string_types = getattr(
19+ types, 'StringTypes', # Python 2
20+ (str, bytes) # Python 3
21+ )
22+ should_wrap = (
23+ isinstance(locations, string_types) or
24+ not isinstance(locations, collections.Iterable))
25+ if should_wrap:
26 locations = [locations]
27
28 for location in locations:

Subscribers

People subscribed via source and target branches