Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update settings description #2928

Merged
merged 1 commit into from
Nov 27, 2022
Merged

Update settings description #2928

merged 1 commit into from
Nov 27, 2022

Conversation

karbobc
Copy link
Contributor

@karbobc karbobc commented Nov 21, 2022

Update description from login to admin in admin page.

Read-Only Config
image

src/config.rs Outdated
@@ -545,7 +545,7 @@ make_config! {

/// Seconds between admin requests |> Number of seconds, on average, between admin requests from the same IP address before rate limiting kicks in
admin_ratelimit_seconds: u64, false, def, 300;
/// Max burst size for login requests |> Allow a burst of requests of up to this size, while maintaining the average indicated by `admin_ratelimit_seconds`
/// Max burst size for admin requests |> Allow a burst of requests of up to this size, while maintaining the average indicated by `admin_ratelimit_seconds`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then, it actually should be admin login instead. Because now it implies on all admin requests, which is not the case.

Copy link
Contributor Author

@karbobc karbobc Nov 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, it should not be change or change to admin login requests?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the latter

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this probably needs to be changed in the template:

## Number of seconds, on average, between admin requests from the same IP address before rate limiting kicks in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll update it at the same time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this probably needs to be changed in the template:

## Number of seconds, on average, between admin requests from the same IP address before rate limiting kicks in.

This doesn't seem to need an update.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not? It says admin, and not admin login. Would be best to keep both items consistent not?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I will update it later.

Update description to `admin login requests`.
@dani-garcia dani-garcia merged commit 39ae2f1 into dani-garcia:main Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants