[WIP] memcache for GAE migration #74
Conversation
richardimaoka
changed the title
| #cache = cache.Redis(**cfg.section('REDIS_INFO')) | ||
| cache = cache.Memcache(**cfg.section('MEMCACHE')) | ||
| ``` | ||
|
|
There was a problem hiding this comment.
切り替えをソースコードの変更に頼っちゃ駄目ですー。環境変数で切り替えがベターだと思います。
There was a problem hiding this comment.
config.json に
"REDIS_INFO"か"MEMCACHE"どちらかを必ず指定しなくてはいけない- どちら「も」指定するとException
- どちら「もない」と、当然Exception
というようにapp.pyを書き換えました。これでどうでしょう？
There was a problem hiding this comment.
えー。あんまり聞いたことのない処理な感じ…
2箇所（config部分と実際のオブジェクト作る部分）で同じようなチェックをしてるのも気になる。
backend = os.getenv('CACHE_BACKEND', 'Redis')
if backend == 'Redis':
cache = cache.Redis(...)
elif backend == 'Memcached':
cache = cache.Memcached(...)
else:
raise Exception('Unknown backend "%s"' % backend)でよくないです？
There was a problem hiding this comment.
なるほど、こちらのほうがきれいですね。ご指摘の通りに変えました。
環境変数はapp.yamlで指定していますので、GAEのときのみ(local dev serverも含めて)memcachedになります。
+env_variables:
- CACHE_BACKEND: 'Memcached'
richardimaoka
force-pushed
the
richard-gae-poc
branch
2 times, most recently
from
11d73ce to
abcc5cd
Compare
| self.client .set(key, val, expires) | ||
|
|
||
| def get(self, key): | ||
| thing = self.client .get(key) |
There was a problem hiding this comment.
これ、
return self.client.get(key)でいいのでは
There was a problem hiding this comment.
新しいコミットで変えました。
|
force pushはPRの中ではしょうがない部分もあるけど、コミット分ける癖をつけておいたほうがいいです！w レビューしていくと、変更した証拠ごと消えていくのちょっと気持ち悪いｗ |
|
はい。すいません。今後は指摘に対する変更はamendでコミットしないで新たなコミットにするようにします。 (「基本1PR 1コミットで」という事を別のところで言われたので、大体どこもそんな文化なのかと…) まだ作業中です。ready for reviewになったら再びコメントでお知らせします。 |
|
1コミット説は僕は眉唾ですね。よしんば１コミットだったとしても、squash mergeとかの結果でないと、意図が読めなくて辛い。（ちなみにsquash mergeも好きではないです） |
lestrrat
changed the title
richardimaoka
force-pushed
the
richard-gae-poc
branch
from
abcc5cd to
a6e6c5c
Compare
|
ready for reviewです。 |
| class Memcached: | ||
| def __init__(self, servers=[], debug=0): | ||
| if os.getenv('SERVER_SOFTWARE', '').startswith('Google App Engine/') or os.getenv('SERVER_SOFTWARE', '').startswith('Development/'): | ||
| print "GAE memcache used" |
|
Ready for review againです。 |
Refs #37 現状の動作には影響のない変更になっているはずです。
memcacheの動作はローカルのvagarnt, GAE local serverで確認しました。