Re-enabled lenient mode #110
Conversation
limitador-server/src/main.rs
Outdated
| @@ -210,6 +210,9 @@ impl Limiter { | |||
| Self::Blocking(limiter) => limiter.configure_with(limits)?, | |||
| Self::Async(limiter) => limiter.configure_with(limits).await?, | |||
| } | |||
| if limitador::limit::check_deprecated_syntax_usages_and_reset() { | |||
| error!("You are using deprecated syntax for your condition! See guide https://kudrant.io/migration/limitador/constraints") | |||
There was a problem hiding this comment.
See guide
See how to migrate with this guide?
yes my suggestion was allowing it be executed as a CLI tool so that it could be used in build etc pipelines |
| @@ -4,14 +4,14 @@ | |||
| max_value: 10 | |||
| seconds: 60 | |||
| conditions: | |||
| - "req.method == 'GET'" | |||
There was a problem hiding this comment.
For the examples, I would not use the deprecated syntax
| @@ -210,6 +210,9 @@ impl Limiter { | |||
| Self::Blocking(limiter) => limiter.configure_with(limits)?, | |||
| Self::Async(limiter) => limiter.configure_with(limits).await?, | |||
| } | |||
| if limitador::limit::check_deprecated_syntax_usages_and_reset() { | |||
There was a problem hiding this comment.
if the feature is not enabled, this would not compile
There was a problem hiding this comment.
Sure... but the feature is always enabled. The way I've done this is the crate has the feature, and this enables it always as a consumer of the crate.
Are you asking I do make the feature available at the binary's layer too (like I did for infinispan)? Or did you think I had made the lenient_conditions a feature at both layers?
There was a problem hiding this comment.
ok I see.
The feature belongs to the limitador crate. And the limitador crate compiles regardless of being activated or not. However, limitador-server package needs it activated or it would not compile.
LGTM
There was a problem hiding this comment.
LGTM
Regarding the "ugly" static switch, It is ok as it is, but I would have added one error per condition, possibly flooding the log. IMO it is a fair toll to pay when using deprecated syntax.
Eventually, in the next release or after, when the deprecated syntax is dropped (or entirely changed by a new struct), this "hack" can be removed
So, to stderr from the crate itself directly? Or have a static
Yes, this goes away in a minor release to come, totally! |
This re-adds the support for the "deprecated"/old syntax for conditions:
ERRORlevel, as this is the default level):The address is:
Should we add a "non-lenient" parser option, so the user can validate their newer files? Or make a "only validate limit file" option, that will not start the server but only parse the
limits file… so that can stay even after this lenient mode gets deprecated, I think @maleck13 proposed something like that actually…