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

Update cookie_store dependency #1268

Merged
merged 6 commits into from Oct 18, 2021
Merged

Update cookie_store dependency #1268

merged 6 commits into from Oct 18, 2021

Conversation

pfernie
Copy link
Sponsor Contributor

@pfernie pfernie commented May 10, 2021

The newly release v0.14.0 of cookie_store splits the implementation of the reqwest::cookie::CookieStore trait for cookie_store::CookieStore into a new separate crate reqwest_cookie_store. This resolves the cyclic dependency described in #1231

This also adds a comment mentioning the new crate in the reqwest::cookie::Jar documentation to suggest users look into this crate for usages not covered by Jar.

@pfernie
Updated cookie_store dependency
8f2056b 
* Bump `cookie_store` version to `0.14.0`
* Add documentation in `cookie` module to direct users to the
  new `reqwest_cookie_store` crate for more advanced scenarios.
@pfernie
update cookie dependency to 0.15
a9dabc3
@pfernie
Update for cookie_store v0.14.1
6de2a84 
* Replace usage of deprecated `cookie_store::CookieStore::get_request_cookies`
  for `cookie_store::CookieStore::get_request_values`.
@pfernie
Update cookie_store to v0.15.0
197dbb8 
The deprecation of `get_request_cookies` should have warranted a
minor version bump.
@pfernie
Merge branch 'master' into cookie_store_14
f674e6d 
* master:
  add option to disable http2 upgrade (seanmonstar#1292)
  Fix documentation of http1_title_case_headers() (seanmonstar#1294)
  CI: make a single final job that depends on all others (seanmonstar#1291)
  Fix bare url warnings in `multipart` docs (seanmonstar#1289)
  v0.11.4
  Allow overriding of DNS resolution to specified IP addresses(seanmonstar#561) (seanmonstar#1277)
  WASM: Add `try_clone` implementations to `Request` and `RequestBuilder` (seanmonstar#1286)
  Add native-tls-alpn feature (seanmonstar#1283)
ducaale added a commit to ducaale/xh that referenced this pull request Sep 13, 2021
@ducaale
vendor reqwest_cookie_store to avoid cyclic dependency
b7de845 
this is needed until seanmonstar/reqwest#1268
gets merged
@pfernie
Copy link
Sponsor Contributor Author

pfernie commented Oct 9, 2021

Are there any changes or additional detail you would like in regards to this PR? It addresses a cyclic dependency issue described in #1231 . The PR brings the cookie_store dependency from 0.12 up to the current 0.15, as well as updating the cookie dependency from 0.14 to 0.15. For cookie_store, this version bump would include breaking changes for users, however the breaking change was in 0.13 -> 0.14, while reqwest had maintained a dependency on 0.12. So leaving this change to merge with the next reqwest release with breaking changes is fine with me, although I'm not sure strictly necessary.

As a note, the reqwest::ClientBuilder api still provides both the legacy cookie_store(bool) as well as cookie_provider<C: CookieStore>(cookie_store: Arc<C>) methods, the former for backwards compatibility. Deprecating the former on the next breaking change release would seem reasonable to me, to narrow the API again.

seanmonstar
seanmonstar approved these changes Oct 18, 2021
View reviewed changes
Copy link
Owner

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

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

Oh wonderful! Sorry I missed this.

@seanmonstar seanmonstar merged commit 1365698 into seanmonstar:master Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seanmonstar seanmonstar seanmonstar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@pfernie @seanmonstar