-
Notifications
You must be signed in to change notification settings - Fork 27
Switch for turning caching on/off for /users/{userid} #280
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
base: development
Are you sure you want to change the base?
Switch for turning caching on/off for /users/{userid} #280
Conversation
The key should be "users:{userId}" - don't use the universal key - that might be a leftover from other code. You want to build the key each time you cache, by passing in the userId, so you can make something this |
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.
Blocking: the redis key for each of these must be "users:{userId}" not "users:all"
Requested: verify the CACHE_TOGGLE key. Your code works and is clean & easy to read, but my PRs for the cache toggle on /users and /users/{userId}/tasks have already been merged, so they won't work together.
Ok so I just saw that Johart's PR already went through with his CACHE_TOGGLE values as true/false, and then I just checked in Azure and it's been changed to true now, so I guess I was just out of the loop when that got changed and I'll have to make a PR to change my code. Disregard my comments on that note! |
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.
There are bugs here - I'm not sure exactly what it is, but when the cache is set to True, it still says caching is set to false in the output. I stepped through it and it looked like it was getting a None result and throwing errors... It could have to do with the thing I pointed out, but I'm not sure.
except TypeError as e: | ||
logging.critical('Failed to fetch user from cache: ' + e.args[1]) | ||
return None | ||
if CACHE_TOGGLE == "true": |
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.
If you just remove the == "true"
this will work better - sometimes it is True, sometimes true, in the Azure function app, but both of those will evaluate to True if you don't tell it to compare Strings. If that makes sense. So everywhere you have if CACHE_TOGGLE == "true":
to just if CACHE_TOGGLE:
Closes #249
WHAT IT DOES:
Feature switch to turn on and off caching for /users/{user_id}
TO TEST:
Set up
Be sure that you are able to use the VS Code debugger. Troubleshooting this alone was both difficult and time consuming.
Install the Azure Cache extension in VS Code.

functions\local.settings.json
file set up:a. Need values: "ENV_DATABASE_CONNECTION_STRING", "ENV_REDIS_KEY", "ENV_REDIS_HOST", "ENV_REDIS_PORT"
b. Add value:
"CACHE_TOGGLE": "true"
Postman is helpful for making the requests.
Testing
Run the debugger on Python Functions.
In Postman or browser make a get request for user with id 1


Terminal:
VS Code Azure extension in Caches:
(click on users:all)
Return to
functions\local.settings.json
file, set"CACHE_TOGGLE": "false"
and save.NOTE: At this point, I had to exit and reopen VSCode entirely to get the debugger to acknowledge the change in this value.
Run the debugger on Python Functions.
In Postman or browser make a get request for user with id 10

Terminal:
VS Code Azure extension in Caches:

Running this process a third time with the toggle set to "true" will result in a cache clear, and the user you requested being added to the cache.
TIME SPENT: