netromdk / slacker

Slacker - Easy access to the Slack API and admin of workspaces/teams.
MIT License
14 stars 0 forks source link

Add Slack API caching layer #114

Closed 0verbyte closed 5 years ago

0verbyte commented 5 years ago

Fixes #105.

Changes proposed in this pull request:

0verbyte commented 5 years ago

As a second step (or task rather) if the command has enabled caching, a flag should be added dynamically that allows the user to clear the items in the cache.

netromdk commented 5 years ago

Additionally, I like the idea of dynamically making it possible to clear the cache of all or certain commands. As I see it there are two approaches:

  1. We introduce a cache command that can list number of entries per command, and sub-commands to clear cache of all or a selection of commands, or
  2. Each command automatically gets a --clear-cache or similar. But this won't make it easy to clear all cache very easily
0verbyte commented 5 years ago

Looks great. The cache is living in memory only, right?

Yep, the cache is in-memory. If the command allows caching it will have its own TTLCache, cache will not be shared with other commands.

I've made the general improvements around how caching keys are generated and looked up.

Regarding clearing command caches. I agree with both of these approaches, perhaps we implement both? I think it is important that each command has the ability to clear its own cache when running with additional arguments. e.g. if you wanted to clear the cache for users.list so you make a fresh api call and cache the new response without executing two separate commands to clear the cache.

netromdk commented 5 years ago

This is great, thanks!

Regarding clearing command caches. I agree with both of these approaches, perhaps we implement both?

Yeah, that's a good point. Should do that in another PR.