Skip to content
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

fix RestController 500 errors #155

Merged
merged 1 commit into from
Sep 10, 2014
Merged

fix RestController 500 errors #155

merged 1 commit into from
Sep 10, 2014

Conversation

pkit
Copy link
Member

@pkit pkit commented Sep 9, 2014

This problem exists in both the RestController and the ApiController.

*Controller.load_config was returning too early if the
cluster_config was retrieved from memcached. Later in the
controller call chain, if cluster_config is set, config_path is
expected to be set as well, but the load_config call was returning
prematurely before config_path could be set.

This was resulting in intermittent generic 500 errors when exercising
the /api/ URL REST features. (Basically, a reqest would work the first
time, then throw errors until the cache expires--about 60 seconds
later--and then start working again.)

This patch fixes it by properly initializing config_path at all times
We also verify it by adding a variation of an exsiting test that tests the behavior in memecache presence

This problem exists in both the RestController and the ApiController.

*Controller.load_config was returning too early if the
cluster_config was retrieved from memcached. Later in the
controller call chain, if cluster_config is set, config_path is
expected to be set as well, but the load_config call was returning
prematurely before config_path could be set.

This was resulting in intermittent generic 500 errors when exercising
the /api/ URL REST features. (Basically, a reqest would work the first
time, then throw errors until the cache expires--about 60 seconds
later--and then start working again.)

This patch fixes it by properly initializing config_path at all times
We also verify it by adding a variation of an exsiting test that tests the behavior in memecache presence
@pkit
Copy link
Member Author

pkit commented Sep 9, 2014

addresses #151

@@ -1684,6 +1684,7 @@ def HEAD(self, req):
return HTTPNotImplemented(request=req)

def load_config(self, req, config_path):
self.config_path = config_path
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we not caching both config_path and cluster_config as we are in the ApiController?

@pkit
Copy link
Member Author

pkit commented Sep 10, 2014

Because in RestController config_path is taken verbatim from request url and not calculated through container metadata (and accessing container metadata could be expensive).

@larsbutler
Copy link
Member

Alright, cool. Looks good!

larsbutler added a commit that referenced this pull request Sep 10, 2014
fix RestController 500 errors
@larsbutler larsbutler merged commit 3c9c960 into zerovm:swift-2.0 Sep 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants