Wafris / wafris-rb

Wafris Client for Rails and Rack applications
https://wafris.org
Other
101 stars 9 forks source link

Tweak docs to encourage better initializer config #58

Closed swanson closed 11 months ago

swanson commented 11 months ago

Subjective so it won't hurt my feelings if you reject :)

I made these deviations from the instructions in our app config today.

I think calling out WAFRIS_REDIS_URL is a good practice, it encourages people not to reuse REDIS_URL.

We also wrapped the initialize to check if the env var was present -- this prevents any unintended things from happening in test/dev environments and allows us to quickly disable Wafris in Heroku (just delete the env var and dynos will reboot). This also allows us to just install the gem in production environments as well.

rmcastil commented 11 months ago

@swanson love this suggestion. One thing we're going to have to tweak is the messaging when the ENV variable doesn't exist. I can see this playing out two ways:

  1. Gem is disabled and the alert only pops up on startup OR
  2. Gem is disabled and alert pops up on every request

I'm leaning on number two. I would think if you were wanted to disable the gem permanently you would delete the ENV variable and remove the gem from your Gemfile. Thoughts?

swanson commented 11 months ago

I would maybe just raise here: https://github.com/Wafris/wafris-rb/blob/main/lib/wafris.rb#L18 if the ENV variable doesn't exist.

I don't think the gem itself needs to have special handling for the "quick disable" case, I just wanted to share why I added the extra check to the initializer.