-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
client breaking: add initial version of client handlers #449
base: 0.3.x
Are you sure you want to change the base?
Conversation
65079b8
to
54b8813
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #449 +/- ##
==========================================
- Coverage 47.96% 47.32% -0.64%
==========================================
Files 169 173 +4
Lines 6569 6683 +114
==========================================
+ Hits 3151 3163 +12
- Misses 3418 3520 +102 ☔ View full report in Codecov by Sentry. |
54b8813
to
e597285
Compare
use trillium_server_common::async_trait; | ||
|
||
#[async_trait] | ||
pub trait ClientHandler: std::fmt::Debug + Send + Sync + 'static { |
Check warning
Code scanning / clippy
missing documentation for a trait Warning
|
||
#[async_trait] | ||
pub trait ClientHandler: std::fmt::Debug + Send + Sync + 'static { | ||
async fn before(&self, conn: &mut Conn) -> crate::Result<()> { |
Check warning
Code scanning / clippy
missing documentation for a method Warning
Ok(()) | ||
} | ||
|
||
async fn after(&self, conn: &mut Conn) -> crate::Result<()> { |
Check warning
Code scanning / clippy
missing documentation for a method Warning
Ok(()) | ||
} | ||
|
||
fn name(&self) -> Cow<'static, str> { |
Check warning
Code scanning / clippy
missing documentation for a method Warning
use crate::{async_trait, client_handler::ClientHandler, Conn, KnownHeaderName, Result}; | ||
|
||
#[derive(Debug, Default, Copy, Clone)] | ||
pub struct FollowRedirects { |
Check warning
Code scanning / clippy
missing documentation for a struct Warning
e597285
to
e0545e0
Compare
@@ -0,0 +1,63 @@ | |||
use crate::{async_trait, client_handler::ClientHandler, Conn, Error, KnownHeaderName, Result}; | |||
use std::mem; |
Check warning
Code scanning / clippy
unused import: std::mem Warning
} | ||
|
||
impl FollowRedirects { | ||
pub fn new() -> Self { |
Check warning
Code scanning / clippy
missing documentation for an associated function Warning
} | ||
|
||
#[derive(Default, Debug)] | ||
pub struct RedirectHistory(Vec<Url>); |
Check warning
Code scanning / clippy
missing documentation for a struct Warning
@@ -11,7 +11,9 @@ keywords = ["trillium", "framework", "async"] | |||
categories = ["web-programming", "web-programming::http-client"] | |||
|
|||
[features] | |||
json = ["serde_json", "serde", "thiserror"] | |||
default = ["cookies", "json"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please consider making cookies
not be enabled by default. Based on previous experience with tide
, once crates in the ecosystem start depending on trillium-client
, it's really hard to get all of them using default-features = false
, even though many won't need this.
I know that trillium is going for a "just works" API, but having to enable a feature flag to get a more featureful client doesn't seem like too much to ask here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a really useful bit of feedback! My initial inclination was for each client handler be their own crate to make totally clear that it doesn't depend on any private state, but convinced myself to limit the proliferation of tiny trillium-client-*
crates. I think I may have overcorrected in the other direction. I'll change them to opt-in instead of opt-out before release, or possibly even go for the trillium-client-cookies, trillium-client-follow-redirects, trillium-client-cache crates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jbr Perhaps a middle ground might make sense: a single trillium-client-handlers
crate for all the handlers that don't add dependencies, and either a separate crate or feature flag (or both with re-export) for handlers that do add dependencies. That should reduce proliferation.
This looks amazing, and I'm looking forward to trying it out, for things like base URLs and standard headers. |
a126843
to
15029f8
Compare
8f6f77f
to
63e23d1
Compare
3fb8da6
to
ac4ef28
Compare
No description provided.