-
Notifications
You must be signed in to change notification settings - Fork 6.6k
Add support for third-party memcache #600
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
Conversation
Change-Id: I4d4e6e5271f5ec1897f6745d2679e1d9240a450b
memcache_port = os.environ.get('GAE_MEMCACHE_PORT', 11211) | ||
memcache_client = MemcacheClient((memcache_addr, int(memcache_port))) | ||
# Environment variables are defined in app.yaml. | ||
if os.environ.get('USE_GAE_MEMCACHE'): |
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.
USE_APPENGINE_MEMCACHE
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.
Why prefer that when the GAE env vars all use the "GAE" initialism?
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.
they're inconsistent. change this one or the one in app.yaml
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.
Oh, gotcha. Sorry! Fixed.
if os.environ.get('USE_GAE_MEMCACHE'): | ||
MEMCACHE_SERVER = ':'.join( | ||
os.environ.get('GAE_MEMCACHE_HOST', 'localhost'), | ||
os.environ.get('GAE_MEMCACHE_PORT', 11211)) |
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.
why not just check these env vars instead of having a separate env var as a flag?
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.
Because eventually these will always be set, and I don't want code that's using third-party memcache to suddenly start using GAE memcache.
@@ -20,6 +20,6 @@ Start your application: | |||
|
|||
Deploy using `gcloud`: | |||
|
|||
gcloud beta app deploy app.yaml | |||
gcloud app deploy app.yaml |
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.
Just gcloud app deploy
here is fine
Change-Id: I738e3fa89ccf3814199e3f90c47f463df8ea7f0f
Change-Id: Ic6d7468bb6145c92fe95c8e28d29c0cf6259dbd3
R: @JustinBeckwith
/cc @jmdobry @bshaffer @broady @tmatsuo @lesv @remi