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

Support saving/loading the cooke store as JSON #1048

Closed

Conversation

scottschroeder
Copy link
Contributor

This is inspired by some of the code & discussion from #1009

I'm looking for feedback, I don't think this is ready as-is.

API Changes

  • Add a save_cookies_json to the Client
  • Add a load_cookies_json to the ClientBuilder

This sort-of exposes the json format as part of the API. Changes to cookie_store could result in old JSON files being unreadable. Given that this would just be an error, and not an API breakage, I'm not sure if that's a concern.

Questions

1) Do we make reqwest::cookie::CookieStore pub?

  • This would make building simpler, because the caller would be responsible for instantiation & error handling.
  • Unfortunately the underlying cookie_store::CookieStore does not support .clone(). Do we care that load_cookies_json would be done on a CookieStore, but save_cookies_json would be done directly on a Client?

2) Error handling?

  • The cookie_store crate raises either Box<dyn Error> or its own CookieError enum. This PR managed to dodge the issue by having save_cookies_json create an io::Error, and load_cookies_json used the Kind::Builder enum.
  • Are we okay with this strategy? Should we add Kind::Cookie? Other ideas?

3) Arguments for save_cookies_json

  • Right now there are two bool flags for persistent and expired. I think the "right" thing is to only save persistent non-expired cookies, but there are reasons to want more options:
    • for debugging purposes, you might want all cookies
    • for my personal usecase, I need to save the non-persistent cookies to be used later, so I would guess there are many similar APIs where cookies are not tagged properly

@meh
Copy link

meh commented Apr 7, 2021

Any update on this?

@seanmonstar
Copy link
Owner

With #1203 merged, adding the CookieStore trait, it's now possible to create any kind of cookie store, such that you can load or save cookies anywhere, and then provide them to a ClientBuilder. Thus, I think this can be closed.

@seanmonstar seanmonstar closed this Apr 7, 2021
@meh
Copy link

meh commented Apr 7, 2021

Oh, absolutely, didn't know that happened, thanks!

@ShayBox
Copy link

ShayBox commented Aug 27, 2024

Smells like we need a serde-cookie-store crate or something now.

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.

4 participants