Fixed group refresh bug #14
Conversation
ssm_cache/cache.py
Outdated
| @@ -93,7 +93,7 @@ def parameter(self, name): | |||
| """ Create a new SSMParameter by name (or retrieve an existing one) """ | |||
| if name in self._parameters: | |||
| return self._parameters[name] | |||
| parameter = SSMParameter(name) | |||
| parameter = SSMParameter(name, max_age=self._max_age) | |||
There was a problem hiding this comment.
This isn't strictly necessary, especially since it's not a public field
There was a problem hiding this comment.
Yeah, but it sounded like the whole Refreshable class would be inconsistent without it. Imagine we add a new method is_cached(): return self._max_age is not None; to Refreshable in the future. I will expect param.is_cached() to return True even if it's part of a group. So I was trying to keep the data Refreshable consistent.
Indeed, an alternative implementation would require to keep _max_age and _last_refresh_time updated for each individual parameter as well (and not redefining _should_refresh at all). What do you think?
There was a problem hiding this comment.
I am removing this for now, and creating a new issue to eventually improve the "consistency" between individual params and their group (internally).
Fixes #13 (see new test)
FYI @benkehoe