Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Sends custom user agent with cloudflare-rs #1070

Merged
merged 3 commits into from
Apr 7, 2020

Conversation

EverlastingBugstopper
Copy link
Contributor

Fixes #731

src/commands/kv/mod.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@ashleymichal ashleymichal left a comment

Choose a reason for hiding this comment

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

i like the clients split out; i'm not keen on the wrapper struct when we're really only ever manipulating a couple of things, both of which are already defined on the struct we are wrapping.

src/http/v4.rs Outdated Show resolved Hide resolved
@ashleymichal ashleymichal dismissed their stale review February 19, 2020 20:33

don't want to block as i am going out of town

@ashleygwilliams ashleygwilliams modified the milestones: 1.8.0, 1.9.0 Feb 25, 2020
src/http/cf.rs Outdated
)
}

pub fn kv_auth_client(user: &GlobalUser) -> Result<HttpApiClient, failure::Error> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe we rename to long_timeout_auth_client?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah prolly. or we handle config less centrally in another iteration of this refactor

Copy link
Contributor

@ashleymichal ashleymichal left a comment

Choose a reason for hiding this comment

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

this seems fine; there are some naming things i'm not thrilled with but i think that is existing baggage and should be addressed in a different refactor. will create issue.

@@ -46,7 +45,7 @@ pub fn global_config(user: &GlobalUser, verify: bool) -> Result<(), failure::Err
// validate_credentials() checks the /user/tokens/verify endpoint (for API token)
// or /user endpoint (for global API key) to ensure provided credentials actually work.
pub fn validate_credentials(user: &GlobalUser) -> Result<(), failure::Error> {
let client = http::cf_v4_api_client(user, HttpApiClientConfig::default())?;
let client = http::auth_client(user)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

the distinction for this client really is that it is optimized as a client for the cloudflare API, not that it includes auth info, so i'd be inclined to leave this name as it was.

src/http/cf.rs Outdated
)
}

pub fn kv_auth_client(user: &GlobalUser) -> Result<HttpApiClient, failure::Error> {
Copy link
Contributor

Choose a reason for hiding this comment

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

yeah prolly. or we handle config less centrally in another iteration of this refactor

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use wrangler user agent for cloudflare-rs requests
3 participants