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

[feat] Add rate limit messaging on repo list page #3025

Merged

Conversation

rohitvinnakota-codecov
Copy link
Contributor

@rohitvinnakota-codecov rohitvinnakota-codecov commented Jul 17, 2024

Description

The resync job may not fetch latest repo data if the logged in owner is rate limited by github. We show some messaging in that case to give users an explanation as to why.

Screenshot 2024-07-17 at 10 36 31 AM

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

@codecov-qa
Copy link

codecov-qa bot commented Jul 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.28%. Comparing base (89da81a) to head (c8a2e3a).

✅ All tests successful. No failed tests found.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3025   +/-   ##
=======================================
  Coverage   98.28%   98.28%           
=======================================
  Files         928      929    +1     
  Lines       13937    13957   +20     
  Branches     3778     3807   +29     
=======================================
+ Hits        13698    13718   +20     
  Misses        235      235           
  Partials        4        4           
Files Coverage Δ
src/services/user/useOwnerRateLimitStatus.tsx 100.00% <100.00%> (ø)
...rgControlTable/RepoOrgNotFound/RepoOrgNotFound.tsx 100.00% <100.00%> (ø)
Components Coverage Δ
Assets 53.84% <ø> (ø)
Layouts 97.75% <ø> (ø)
Pages 99.03% <ø> (ø)
Services 99.51% <100.00%> (+<0.01%) ⬆️
Shared 99.69% <100.00%> (+<0.01%) ⬆️
UI 93.99% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 89da81a...c8a2e3a. Read the comment docs.

Copy link

codecov-public-qa bot commented Jul 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.28%. Comparing base (89da81a) to head (c8a2e3a).

✅ All tests successful. No failed tests found.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3025   +/-   ##
=======================================
  Coverage   98.28%   98.28%           
=======================================
  Files         928      929    +1     
  Lines       13937    13957   +20     
  Branches     3778     3807   +29     
=======================================
+ Hits        13698    13718   +20     
  Misses        235      235           
  Partials        4        4           
Files Coverage Δ
src/services/user/useOwnerRateLimitStatus.tsx 100.00% <100.00%> (ø)
...rgControlTable/RepoOrgNotFound/RepoOrgNotFound.tsx 100.00% <100.00%> (ø)
Components Coverage Δ
Assets 53.84% <ø> (ø)
Layouts 97.75% <ø> (ø)
Pages 99.03% <ø> (ø)
Services 99.51% <100.00%> (+<0.01%) ⬆️
Shared 99.69% <100.00%> (+<0.01%) ⬆️
UI 93.99% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 89da81a...c8a2e3a. Read the comment docs.

@codecov-notifications
Copy link

codecov-notifications bot commented Jul 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3025   +/-   ##
=======================================
  Coverage   98.28%   98.28%           
=======================================
  Files         928      929    +1     
  Lines       13937    13957   +20     
  Branches     3802     3783   -19     
=======================================
+ Hits        13698    13718   +20     
  Misses        235      235           
  Partials        4        4           
Files Coverage Δ
src/services/user/useOwnerRateLimitStatus.tsx 100.00% <100.00%> (ø)
...rgControlTable/RepoOrgNotFound/RepoOrgNotFound.tsx 100.00% <100.00%> (ø)
Components Coverage Δ
Assets 53.84% <ø> (ø)
Layouts 97.75% <ø> (ø)
Pages 99.03% <ø> (ø)
Services 99.51% <100.00%> (+<0.01%) ⬆️
Shared 99.69% <100.00%> (+<0.01%) ⬆️
UI 93.99% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 89da81a...c8a2e3a. Read the comment docs.

@codecov-staging
Copy link

codecov-staging bot commented Jul 17, 2024

Bundle Report

Changes will increase total bundle size by 1.27kB ⬆️

Bundle name Size Change
gazebo-staging-array-push 5.84MB 1.27kB ⬆️

Copy link

codecov bot commented Jul 17, 2024

Bundle Report

Changes will increase total bundle size by 1.27kB ⬆️

Bundle name Size Change
gazebo-production-array-push 5.84MB 1.27kB ⬆️

Copy link

codecov bot commented Jul 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.28%. Comparing base (89da81a) to head (c8a2e3a).

✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##               main      #3025   +/-   ##
===========================================
  Coverage   98.28000   98.28000           
===========================================
  Files           928        929    +1     
  Lines         13937      13957   +20     
  Branches       3802       3807    +5     
===========================================
+ Hits          13698      13718   +20     
  Misses          235        235           
  Partials          4          4           
Files Coverage Δ
src/services/user/useOwnerRateLimitStatus.tsx 100.00% <100.00%> (ø)
...rgControlTable/RepoOrgNotFound/RepoOrgNotFound.tsx 100.00% <100.00%> (ø)
Components Coverage Δ
Assets 53.84% <ø> (ø)
Layouts 97.75% <ø> (ø)
Pages 99.03% <ø> (ø)
Services 99.51% <100.00%> (+<0.01%) ⬆️
Shared 99.69% <100.00%> (+<0.01%) ⬆️
UI 93.99% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 89da81a...c8a2e3a. Read the comment docs.

@codecov-releaser
Copy link
Contributor

codecov-releaser commented Jul 17, 2024

✅ Deploy preview for gazebo ready!

Previews expire after 1 month automatically.

Storybook

Commit Created Cloud Enterprise
78d7a04 Wed, 17 Jul 2024 15:08:43 GMT Expired Expired
78d7a04 Wed, 17 Jul 2024 15:19:13 GMT Expired Expired
3b9fcd9 Wed, 17 Jul 2024 19:25:18 GMT Expired Expired
3b9fcd9 Wed, 17 Jul 2024 19:29:24 GMT Expired Expired
3b9fcd9 Wed, 17 Jul 2024 19:40:04 GMT Expired Expired
84aa81d Thu, 18 Jul 2024 15:36:51 GMT Expired Expired
5dec827 Thu, 18 Jul 2024 15:58:13 GMT Expired Expired
759dc92 Thu, 18 Jul 2024 16:52:58 GMT Expired Expired
d226b5a Thu, 18 Jul 2024 17:15:31 GMT Expired Expired
837c7a2 Thu, 18 Jul 2024 17:28:28 GMT Expired Expired
c8a2e3a Tue, 30 Jul 2024 13:18:06 GMT Cloud Enterprise

@rohitvinnakota-codecov rohitvinnakota-codecov marked this pull request as ready for review July 17, 2024 18:50
@rohitvinnakota-codecov rohitvinnakota-codecov changed the title [draft] Add rate limit messaging on repo list page [feat] Add rate limit messaging on repo list page Jul 17, 2024
Copy link
Contributor

@nicholas-codecov nicholas-codecov left a comment

Choose a reason for hiding this comment

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

Couple of comments to peak at

Comment on lines 12 to 14
.nullish(),
})
.nullish()
Copy link
Contributor

Choose a reason for hiding this comment

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

m: We want to move these to be nullable() instead of nullish() as GQL cannot return undefined fields, so that's adding some extra burden on our end to handle cases that cannot occur.

const data = parsedData.data

return {
isGithubRateLimited: data?.owner?.isGithubRateLimited,
Copy link
Contributor

Choose a reason for hiding this comment

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

l: Thoughts on assigning this a default value data?.owner?.isGithubRateLimited ?? true | false just to be a bit more explicit on the return type?

Comment on lines 41 to 44
return Promise.reject({
status: 404,
data: {},
})
Copy link
Contributor

Choose a reason for hiding this comment

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

l: Can you add in something along these lines so that we have better debugging messages in case this is thrown.

@codecovdesign
Copy link
Contributor

codecovdesign commented Jul 18, 2024

if we know the user is rate limited (via the hook), why do we still allow the user to resync?

@rohitvinnakota-codecov suggestion is we cut the syncing action/copy in this circumstance and clarify the rate limit issue:

Cant’t find your repo? It may be due to GitHub rate limits

Unless this is confirmed below, then perhaps we keep?

fallback logic for some endpoints that uses a public bot token so it may work

Copy link
Contributor

@nicholas-codecov nicholas-codecov left a comment

Choose a reason for hiding this comment

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

LGTM

@rohitvinnakota-codecov rohitvinnakota-codecov added this pull request to the merge queue Jul 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 30, 2024
@rohitvinnakota-codecov rohitvinnakota-codecov added this pull request to the merge queue Jul 30, 2024
Merged via the queue into main with commit dc77b3d Jul 30, 2024
61 checks passed
@rohitvinnakota-codecov rohitvinnakota-codecov deleted the rvinnakota/update-resync-message-rate-limit branch July 30, 2024 13:27
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.

4 participants