uclahs-cds / Ligare

GNU General Public License v2.0
0 stars 0 forks source link

Abstract `FeatureFlagRouter` and add a `CachingFeatureFlagRouter` #98

Closed aholmes closed 2 months ago

aholmes commented 2 months ago

The base FeatureFlagRouter was doing too much. This PR abstracts its operations into a CachingFeatureFlagRouter that can be used by other feature flag subclasses, or does not have to be used. This allows consumers to rely on the FeatureFlagRouter interface and base class without implementations having potentially unexpected behaviors with the caching operations that used to exist in the base class.

Closes #95

nwiltsie commented 2 months ago

I can take a look this afternoon tomorrow! (This comment always said tomorrow, ignore that edit history.)

dan-knight commented 2 months ago

Can you explain what "caching" means in this context? Is it simply that the feature flags are cached within the running application?

j2salmingo commented 2 months ago

Can you explain what "caching" means in this context? Is it simply that the feature flags are cached within the running application?

It looks like it's cached within a CachingFeatureFlagRouter object, so I suppose it depends on how you use the object

aholmes commented 2 months ago

Can you explain what "caching" means in this context? Is it simply that the feature flags are cached within the running application?

"Caching" here means that the specific CachingFeatureFlagRouter class is a feature flag router that maintains an internal cache of flags and their values. On its own, that means anything it "sets" is then retrieved (when feature_is_enabled is called) without operations outside of itself. This then extends to subclasses of itself that can take advantage of this. In the case of DBFeatureFlagRouter, it uses CachingFeatureFlagRouter's behavior to avoid querying the database every time something needs to know if a flag is enabled or not.

aholmes commented 2 months ago

Looks good! I'll happily debate minute style details and class architecture philosophy as long as you let me, but you're free to disengage at any point.

I appreciate the feedback - after all, these changes came from a team discussion about our coding patterns.

aholmes commented 2 months ago

@nwiltsie I've finished all the changes related to our discussions.