Config param `cache: enabled` results in exception
neslinesli93 opened this issue · 6 comments
Hi!
We've been using your library for some time - it's wonderful, thanks for all the effort you put into it.
We've started noticing a strange pattern when using FunWithFlags for unit tests. Our setup consists of a postgres table that is used as the backend for the library, where all the flags are saved. We opted for the db backend in tests since Phoenix offers isolation for accessing the database during tests, while there is no such thing for redit.
However, the fact that your library implements a caching mechanism via ETS seems to provide shared behaviour between tests, which we want to avoid.
Thus we set the following params inside our test config:
config :fun_with_flags, :cache, enabled: falseAnd we keep getting crashes because the library tries to call the cache genserver anyway... The offending function seems to be this one, which gets called after each operation of reading/saving flags, whatever the value of the :cache property is.
I think it would be pretty easy to check the config before calling Cache.put or Cache.get, in order to avoid crashes... What do you think?
We tried the said fix locally and it seemed to work, even though we had to manually recompile the dependency everytime we made a change, since it seems like the persistent store adapter needs to be available at compile time. Any clue about this?
Thanks again, and keep up with the good work
Hi!
Thank you for using the library, for the appreciation and for opening the issue.
What you describe should not happen, because when the cache is disabled the SimpleStore module should be used instead of Store (the one that implements the function you linked). It's done in Config.store_module/0, which is used to memoize the module.
I believe that you're experiencing this issue. What's happening is that once FunWithFlags is compiled to BEAM bytecode (in YOUR_PROJECT/_build, and the store module has been compiled in, changes to your Mix env won't have effect on it. More details are in the linked paragraph of the readme.
You should be able to solve it with:
rm -r _build/test/lib/fun_with_flags
# rm -r _build/test/lib/YOUR_APP_NAME # Try to also add this one if the one above doesn't help(...) even though we had to manually recompile the dependency everytime we made a change, since it seems like the persistent store adapter needs to be available at compile time. Any clue about this?
Yes, that's exactly the same issue.
That's an implementation detail that I'm not particularly happy with, and I must confess that when I wrote it I wasn't very familiar yet with how Elixir compilation work. I've been actually been meaning to fix that, and I have a local branch where I remove those compiled-in module references, but I'm not actively working on it at the moment. It's something that I want to be careful about because compiling in the modules does make things a bit faster, so I want to run some proper benchmarks before committing to it.
We've started noticing a strange pattern when using FunWithFlags for unit tests.
(...)
However, the fact that your library implements a caching mechanism via ETS seems to provide shared behaviour between tests, which we want to avoid.
Anyway, if that's the problem you can also use Store.Cache.flush/0. It's meant to help with this exact use case!
Some example of how it's used in FWF's own test suite:
- https://github.com/tompave/fun_with_flags/blob/v1.4.0/test/support/test_utils.ex#L43-L47
- https://github.com/tompave/fun_with_flags/blob/v1.4.0/test/fun_with_flags_test.exs#L9-L21
- https://github.com/tompave/fun_with_flags/blob/v1.4.0/test/fun_with_flags/store_test.exs#L20
- https://github.com/tompave/fun_with_flags/blob/v1.4.0/test/fun_with_flags/store/cache_test.exs#L10
Another thing:
We've started noticing a strange pattern when using FunWithFlags for unit tests. Our setup consists of a postgres table that is used as the backend for the library, where all the flags are saved. We opted for the db backend in tests since Phoenix offers isolation for accessing the database during tests, while there is no such thing for redit.
Does that mean that you use the Redis store in prod and dev, but the Postgres store in the tests just because of the SQL sandbox (auto-rollback of DB transactions)?
If that's the case, then you could write something similar to this test helper, which is used to clear Redis in the unit tests of FWF:
Example usage:
Thank you so much for the detailed response!
Does that mean that you use the Redis store in prod and dev, but the Postgres store in the tests just because of the SQL sandbox (auto-rollback of DB transactions)?
Yes, that's exactly our setup. I think we'll end up flushing the ETS cache in our setup functions, which seems to be the best solution so far, very close to disabling the cache entirely in tests...
Thanks, and keep up with the good work
Closing as solved.