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

Rate limit getting device logs #1430

Closed
wants to merge 1 commit into from

Conversation

fisehara
Copy link
Contributor

Opening a device logs stream or retrieving restfull device logs is not rate limited.

Rate limiter distinguishes between stream and document

Change-type: minor

@fisehara fisehara force-pushed the fisehara/rate-limit-device-logs-stream branch from 36dc682 to 972a1a6 Compare September 14, 2023 23:17
@flowzone-app flowzone-app bot enabled auto-merge September 14, 2023 23:19
test/06_device-log.ts Outdated Show resolved Hide resolved
test/06_device-log.ts Outdated Show resolved Hide resolved
test/06_device-log.ts Outdated Show resolved Hide resolved
test/06_device-log.ts Outdated Show resolved Hide resolved
@fisehara fisehara force-pushed the fisehara/rate-limit-device-logs-stream branch from 972a1a6 to f31e616 Compare September 15, 2023 07:06
@fisehara fisehara requested a review from joshbwlng September 15, 2023 07:07
Opening a device logs stream or retrieving restfull device logs is not rate limited.

Rate limiter distinguishes between stream and document

Change-type: minor
Signed-off-by: Harald Fischer <[email protected]>
@fisehara fisehara force-pushed the fisehara/rate-limit-device-logs-stream branch from f31e616 to b2dc7f3 Compare September 15, 2023 07:23
@fisehara fisehara requested a review from thgreasi September 15, 2023 09:40
fieldFn = (req) =>
fields
.map((field) => {
const path = _.toPath(field);
Copy link
Contributor

Choose a reason for hiding this comment

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

This _.toPath must occur only once in the setup phase as it is fairly costly when in very hot code (eg in rate limiting code that potentially runs for every request). If we didn't care about pre-calculating the path in order to do it only once then we would instead do _.get(req, field); and let lodash do it automatically each time

@fisehara
Copy link
Contributor Author

Rate limiter is in place, during tests rate limiting is intentionally relaxed. Which enforcing strict rate limiting the endpoints get limited, generously but limited.

@fisehara fisehara closed this Sep 27, 2023
auto-merge was automatically disabled September 27, 2023 15:24

Pull request was closed

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