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

Add multiple cookies #95

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jkone27
Copy link

@jkone27 jkone27 commented Jan 26, 2022

add ability to add multiple cookies in a single step, from a list of cookie strings

add ability to add multiple cookies in a single step, from a list of cookie strings
@SchlenkR SchlenkR self-requested a review January 26, 2022 10:07
Copy link
Member

@SchlenkR SchlenkR left a comment

Choose a reason for hiding this comment

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

  • In general, the functions in DslCE are basically mirrored from the functions in Dsl. The implementation is in Dsl module and the DslCE functions are forwarding calls to their corresponding Dsl function.
  • What is the benefit of a single Cookies operation passing a seq of cookies in comparison to use multiple single-Cookie statements? I don't see a big advantage here - maybe a little less writing - but some downsides:
    • There are 3 overloads for single cookies that clarify the available parameters and their meaning. Having just one Cookies operation that gets a sequence of strings will hide that.
    • Writing multiple single-cookie statements are close to how a raw HTTP request looks like.

@SchlenkR
Copy link
Member

Related to #2

@jkone27
Copy link
Author

jkone27 commented Jan 31, 2022

yes i agree, maybe is not needed but indeed was linked to trying to read Accept-Cookies from response headers and place them all in the new request in 1 go

@SchlenkR
Copy link
Member

SchlenkR commented Feb 1, 2022

... trying to read Accept-Cookies from response headers ...

The response header I know is Set-Cookie - is it that what is meant here?

If so, it would propable be useful to change the signature to accepting either

  • a System.Net.Cookie seq, and / or
  • a System.Net.Http.HttpResponseMEssage, and / or
  • a FsHttp.Domain.Response

Do you have a working example (maybe with an available online test service)?

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.

None yet

2 participants