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.

Sign up for GitHub

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

Jump to bottom

🌱Add handleRateLimit method #1387

Merged
merged 1 commit into from
Aug 23, 2024
Merged

🌱Add handleRateLimit method #1387

merged 1 commit into from
Aug 23, 2024

Conversation

yrs147
Copy link
Contributor

@yrs147 yrs147 commented Jul 17, 2024

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1340

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

  • squash commits
  • include documentation
  • add unit tests

@yrs147 yrs147 requested a review from janiskemper July 17, 2024 06:22
janiskemper
janiskemper requested changes Jul 17, 2024
View reviewed changes
pkg/services/hcloud/server/server.go Outdated Show resolved Hide resolved
pkg/services/hcloud/server/server.go Show resolved Hide resolved
pkg/services/hcloud/server/server.go Outdated Show resolved Hide resolved
@yrs147 yrs147 requested a review from janiskemper July 18, 2024 08:40
janiskemper
janiskemper requested changes Jul 29, 2024
View reviewed changes
Copy link
Contributor

@janiskemper janiskemper left a comment

Choose a reason for hiding this comment

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

let's additionally do the following: If the machine is supposed to be deleted (i.e. has a deletion timestamp), then let's also show the error in the condition.

@janiskemper janiskemper requested a review from guettli August 1, 2024 08:07
@janiskemper
Copy link
Contributor

LGTM from my side. What do you think @guettli ?
@yrs147 did you make sure that linting and everything works?

@yrs147
Copy link
Contributor Author

yrs147 commented Aug 1, 2024

LGTM from my side. What do you think @guettli ? @yrs147 did you make sure that linting and everything works?

@janiskemper yes I have

@yrs147 yrs147 requested a review from janiskemper August 12, 2024 08:46
@yrs147 yrs147 force-pushed the yrs/fix-rate-limit branch from 1ebd19a to 6e2559c Compare August 12, 2024 13:25
janiskemper
janiskemper reviewed Aug 12, 2024
View reviewed changes
pkg/services/hcloud/server/server_test.go Outdated Show resolved Hide resolved
pkg/services/hcloud/server/server_test.go Outdated Show resolved Hide resolved
pkg/services/hcloud/server/server_test.go Outdated Show resolved Hide resolved
@yrs147 yrs147 force-pushed the yrs/fix-rate-limit branch from 7661ac1 to f185a46 Compare August 12, 2024 17:38
janiskemper
janiskemper requested changes Aug 13, 2024
View reviewed changes
pkg/services/hcloud/server/server_test.go Outdated Show resolved Hide resolved
pkg/services/hcloud/server/server_test.go Outdated Show resolved Hide resolved
@guettli guettli mentioned this pull request Aug 13, 2024
Closed
@yrs147 yrs147 requested a review from janiskemper August 14, 2024 09:21
janiskemper
janiskemper reviewed Aug 14, 2024
View reviewed changes
Copy link
Contributor

@janiskemper janiskemper left a comment

Choose a reason for hiding this comment

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

LGTM you can squash

@yrs147 yrs147 force-pushed the yrs/fix-rate-limit branch from 21fd624 to 8023082 Compare August 14, 2024 09:39
@yrs147 yrs147 marked this pull request as ready for review August 14, 2024 09:40
@syself-bot syself-bot bot added the area/code Changes made in the code directory label Aug 14, 2024
@yrs147 yrs147 requested a review from janiskemper August 14, 2024 10:57
@janiskemper janiskemper requested a review from guettli August 14, 2024 14:13
guettli
guettli requested changes Aug 15, 2024
View reviewed changes
pkg/services/hcloud/server/server.go Outdated Show resolved Hide resolved
pkg/services/hcloud/server/server.go Outdated Show resolved Hide resolved
@syself-bot syself-bot bot added the size/M Denotes a PR that changes 50-200 lines, ignoring generated files. label Aug 16, 2024
@batistein
Copy link
Contributor

@yrs147 please address the requests on monday so we can get this merged

@yrs147 yrs147 requested a review from guettli August 21, 2024 11:34
guettli
guettli approved these changes Aug 21, 2024
View reviewed changes
Copy link
Collaborator

@guettli guettli left a comment

Choose a reason for hiding this comment

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

lgtm.

@yrs147 yrs147 force-pushed the yrs/fix-rate-limit branch from 2d7b5f7 to ce0da02 Compare August 21, 2024 12:51
@batistein batistein merged commit ddcac0a into main Aug 23, 2024
9 checks passed
@batistein batistein deleted the yrs/fix-rate-limit branch August 23, 2024 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guettli guettli guettli approved these changes

@janiskemper janiskemper Awaiting requested review from janiskemper

Assignees
No one assigned
Labels
area/code Changes made in the code directory size/M Denotes a PR that changes 50-200 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Don't show rate limit errors on running machines
4 participants
@yrs147 @janiskemper @batistein @guettli