Monitor config file for changes and reload when needed

[alexsnaps]

See issue #74

What has changed

• bafcb99 Adds the behavior to reload the limit file off disk when it changed and do the changes
• 4394736 Returns proper errors when handling the limit file off disk

Testing it locally

1. Build locally
   $ cargo build
2. Start limitador server, using the example config
   $ LIMITS_FILE=limitador-server/examples/limits.yaml RUST_LOG=info ./target/debug/limitador-server
3. Hit the HTTP end point to list limits at:
   http://localhost:8080/limits/test_namespace
4. Change a value in the config file at limitador-server/examples/limits.yaml and save
5. Reload the URL and the changes should be visible
6. An invalid config file would trigger an error to be logged and be ignored (i.e. the old config remains active)

[eguzki]

Nice! the configure_with that was "reconciling" limits in the external storage, now it is a perfect match for the in-memory limits.

[eguzki]

owesome!

[eguzki]

nothing to be done with the watcher? maybe register a signal to stop watching?

[alexsnaps]

Well that's what this does… RAII… i.e. we keep it around, and things get cleaned up upon drop(), e.g. in the case of linux, that'd be here.

.drop() is automatically invoked when the thing gets out of scope and can safely be dropped. Not keeping it around, it be dropped immediately…

[alexsnaps]

btw, this is very much like deferal as you'd know it from golang… only it's automatic so that you cannot forget to tell it to release the resources!

[eguzki]

Ok, let me verify if I understood.

When main() function run, the execution point is blocked at

run_http_server(&http_api_address, rate_limiter.clone()).await?;

When the process gets killed (with a signal), the execution continues in main returning Ok(()). Then, _watcher which is an Option holding the watcher, gets out of scope and dropped. Notify lib will clean up at that moment.

is that right?

[alexsnaps]

Yes that's exactly it. Or to be real pedantic, while you hit the return, the function doesn't return until .drop() has itself returned on the Option (that if it owns the Watcher eventually delegates to its drop())…

The doc might provide more clarity than my explanation here… otherwise (and I recommend it in all cases) Rust for Rustaceans is excellent, and the free first chapter has the part on Drop and drop order on page 9!

[eguzki]

I wonder if this is the same as

let limiter = rate_limiter.clone();

[alexsnaps]

As per the documentation, it is indeed:

// The two syntaxes below are equivalent.
let a = foo.clone();
let b = Arc::clone(&foo);

Dunno why I used one over the other… which ever you prefer ¯_(ツ)_/¯

[eguzki]

👍 thanks for the doc ref

[eguzki]

Two more questions:

• The watcher is waiting for EventKind::Modify events. What happens when the process starts? The file is already there but not changed. Does the modify even trigger?
• If I do not get it wrong, the LIMITS_FILE_ENV env var is optional, and if it does not exist, the watcher will not start. Effectively, the limits will be empty forever (forever is the lifetime of the process). I wonder if we need to enforce the existence of the env var and if not, exit the process with an error.

[alexsnaps]

Two more questions:

* The watcher is waiting for `EventKind::Modify` events. What happens when the process starts? The file is already there but not changed. Does the modify even trigger?

No the event triggers only when observed.

* If I do not get it wrong, the `LIMITS_FILE_ENV` env var is optional, and if it does not exist, the watcher will not start. Effectively, the limits will be empty forever (forever is the lifetime of the process). I wonder if we need to enforce the existence of the env var and if not, exit the process with an error.

That's what the Option is for. If you did not provide a LIMITS_FILE env var then yes, no watcher… but most importantly you effectively have a useless limitador instance 🤔 So I think you might be right there!

Do you want to do this as part of this PR? I have one question about this tho: should we have a default? If so which?
And now I really start hating the idea of a mandatory env variable and would much rather provide this as an argument to the process… wdyt?

I'd create another issue & PR for this, with #78 being merged… this is the behavior you'd get running HEAD of main already…

[eguzki]

Do you want to do this as part of this PR? I have one question about this tho: should we have a default? If so which?
And now I really start hating the idea of a mandatory env variable and would much rather provide this as an argument to the process… wdyt?

I would say a different PR.

For the remaining questions: I agree that and env var should always be optional. The main question is, do we want to have this limits file path as a required input to the process? If yes, then I would go to command line arg. The libraries to manage command line args usually have clear mechanisms to flag some command line arg as required. It could even be a positional param instead of a optional param (e.g. --limits-file).
On the other hand, if limitador defines a default file as the source of limits (for example /opt/limitador/etc/limits.yaml), then I really do not care about being via env var or optional param (e.g. --limits-file) the mechanism to override the default one.

[alexsnaps]

Looks like we have a similar view on this :)
Created #83 to track this… i.e. make a decision and impl. it.