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

Unifty httpd in blackholes #1187

Merged
merged 5 commits into from
Dec 30, 2024
Merged

Unifty httpd in blackholes #1187

merged 5 commits into from
Dec 30, 2024

Conversation

blt
Copy link
Collaborator

@blt blt commented Dec 27, 2024

What does this PR do?

This commit unifies the httpd bits of the blackholes, removing a duplicated loop in three places. The type logic here started out general and got gradually more concrete and there's scope to reduce duplication even further but I don't know that it's a pressing issue.

Copy link
Collaborator Author

blt commented Dec 27, 2024

@blt blt added the no-changelog label Dec 27, 2024 — with Graphite App
@blt blt marked this pull request as ready for review December 27, 2024 23:28
@blt blt requested a review from a team as a code owner December 27, 2024 23:28
@blt blt force-pushed the blt/re-unify_crates_at_workspace branch from 977e2c1 to 8c123ec Compare December 30, 2024 15:47
@blt blt force-pushed the blt/unifty_httpd_in_blackholes branch from eaf3562 to ed698ac Compare December 30, 2024 15:47
Copy link
Contributor

@scottopell scottopell left a comment

Choose a reason for hiding this comment

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

Change looks good for unifying the common functionality here

where
// "service factory"
SF: Send + Sync + 'static + Clone + Fn() -> S,
// The bounds on `S` per
Copy link
Contributor

Choose a reason for hiding this comment

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

helpful comments, thanks

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I found it very hard to keep straight myself.

lading/src/blackhole/common.rs Outdated Show resolved Hide resolved
lading/src/blackhole/common.rs Outdated Show resolved Hide resolved
@blt blt force-pushed the blt/re-unify_crates_at_workspace branch from 8c123ec to 086520b Compare December 30, 2024 17:15
@blt blt force-pushed the blt/unifty_httpd_in_blackholes branch from ed698ac to 72aad51 Compare December 30, 2024 17:15
@blt blt force-pushed the blt/re-unify_crates_at_workspace branch 2 times, most recently from bbac7b4 to 6344aba Compare December 30, 2024 17:52
@blt blt force-pushed the blt/unifty_httpd_in_blackholes branch from a1e268c to 6c7cfc1 Compare December 30, 2024 17:52
@blt blt force-pushed the blt/re-unify_crates_at_workspace branch from 6344aba to 425c381 Compare December 30, 2024 18:01
@blt blt force-pushed the blt/unifty_httpd_in_blackholes branch from 6c7cfc1 to aa1d0a8 Compare December 30, 2024 18:01
Copy link
Contributor

@GeorgeHahn GeorgeHahn left a comment

Choose a reason for hiding this comment

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

Catching up to the slack thread on this - I think this is functionally identical to the behavior of the previous tower concurrency limit + load shed combination.

@blt blt changed the base branch from blt/re-unify_crates_at_workspace to graphite-base/1187 December 30, 2024 18:36
@blt blt force-pushed the graphite-base/1187 branch from 425c381 to c7aa607 Compare December 30, 2024 18:37
@blt blt force-pushed the blt/unifty_httpd_in_blackholes branch from aa1d0a8 to 6810a8b Compare December 30, 2024 18:37
@blt blt changed the base branch from graphite-base/1187 to main December 30, 2024 18:37
@blt blt force-pushed the blt/unifty_httpd_in_blackholes branch from 6810a8b to d899db2 Compare December 30, 2024 18:37
blt added 5 commits December 30, 2024 10:44
This commit unifies the httpd bits of the blackholes, removing a duplicated
loop in three places. The type logic here started out general and got
gradually more concrete and there's scope to reduce duplication even further
but I don't know that it's a pressing issue.

Signed-off-by: Brian L. Troutwine <[email protected]>
Signed-off-by: Brian L. Troutwine <[email protected]>
Signed-off-by: Brian L. Troutwine <[email protected]>
Signed-off-by: Brian L. Troutwine <[email protected]>
@blt blt merged commit b2f3c8b into main Dec 30, 2024
18 checks passed
Copy link
Collaborator Author

blt commented Dec 30, 2024

Merge activity

  • Dec 30, 2:21 PM EST: A user merged this pull request with Graphite.

@blt blt deleted the blt/unifty_httpd_in_blackholes branch December 30, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants