Skip to content

fix: ratelimit - updates#10347

Merged
PeerRich merged 2 commits into
mainfrom
fix/expand-ratelimit
Jul 27, 2023
Merged

fix: ratelimit - updates#10347
PeerRich merged 2 commits into
mainfrom
fix/expand-ratelimit

Conversation

@sean-brydon

Copy link
Copy Markdown
Member

Adds a few changes for rate limiting

@vercel

vercel Bot commented Jul 24, 2023

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
api ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 24, 2023 9:44pm
cal-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 24, 2023 9:44pm
dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 24, 2023 9:44pm
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Jul 24, 2023 9:44pm
qa ⬜️ Ignored (Inspect) Jul 24, 2023 9:44pm
ui ⬜️ Ignored (Inspect) Visit Preview Jul 24, 2023 9:44pm

@github-actions

github-actions Bot commented Jul 24, 2023

Copy link
Copy Markdown
Contributor

Thank you for following the naming conventions! 🙏

@zomars zomars added the core area: core, team members only label Jul 24, 2023
Comment thread packages/lib/rateLimit.ts
Comment on lines +60 to +65
common: new Ratelimit({
redis,
analytics: true,
prefix: "ratelimit",
limiter: Ratelimit.fixedWindow(200, "60s"),
}),

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

New limiting type that will be used across higher called apis

@github-actions

github-actions Bot commented Jul 24, 2023

Copy link
Copy Markdown
Contributor

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@deploysentinel

deploysentinel Bot commented Jul 24, 2023

Copy link
Copy Markdown

Current Playwright Test Results Summary

✅ 74 Passing - ⚠️ 1 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 07/24/2023 09:49:04pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 96fe263

Started: 07/24/2023 09:47:19pm UTC

⚠️ Flakes

📄   apps/web/playwright/login.2fa.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
2FA Tests should allow a user to enable 2FA and login using 2FA
Retry 1Initial Attempt
9.90% (31) 31 / 313 runs
failed over last 7 days
36.42% (114) 114 / 313 runs
flaked over last 7 days

View Detailed Build Results


@alwaysmeticulous

alwaysmeticulous Bot commented Jul 24, 2023

Copy link
Copy Markdown

🤖 Meticulous spotted visual differences in 37 of 228 screens tested: view and approve differences detected.

Last updated for commit 96fe263. This comment will update as new commits are pushed.

Comment thread packages/lib/rateLimit.ts
prefix: "ratelimit",
limiter: Ratelimit.fixedWindow(10, "60s"),
}),
common: new Ratelimit({

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
common: new Ratelimit({
common: () => new Ratelimit({

We should invoque the limiters otherwise we're loading all of them instead of only the ones needed. Not a big deal since there's only 3 but a good practice for scaling

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Getting some errors doing this - will create issue n follow up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants