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

WIP #817

Merged
merged 1 commit into from
Sep 27, 2024
Merged

WIP #817

merged 1 commit into from
Sep 27, 2024

Conversation

dancoombs
Copy link
Collaborator

[Closes/Fixes] #

Proposed Changes

);

let start = Instant::now();
let mut svc = self.inner.clone();
let service_name = self.service_name.clone();
let protocal = self.protocal.clone();
let protocal = self.protocol.clone();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Typo here still protocol

}
}

impl<T, R> Clone for MetricsLayer<T, R>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Needed to manually implement clone because derive(Clone) requires that T, R be Clone, where they don't need to be since we're using phantom data.

_request_type: PhantomData<R>,
}

impl<S, T, R> Clone for MetricsMiddleware<S, T, R>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same reason here

Comment on lines +117 to +120
S: Service<R> + Send + Clone + 'static,
S::Future: Send + 'static,
T: RequestExtractor<R> + 'static,
R: Send + 'static,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed all of the Sync from these bounds as its not required by Service. Tbh I'm unsure why but this made the compiler happy, example here https://github.com/hyperium/tonic/blob/master/examples/src/tower/server.rs

Comment on lines +147 to +149
&method_name,
&service_name,
&protocal,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Very common to convert String -> &str with a dereference

@andysim3d andysim3d merged commit 0ee1142 into andy/fix-middleware Sep 27, 2024
2 of 7 checks passed
@andysim3d andysim3d deleted the danc/fix-middleware-diff branch September 27, 2024 17:13
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.

2 participants