Skip to content

Handle cookies dict set to prevent AttributeError#91

Open
rojoyin wants to merge 1 commit intodaijro:mainfrom
rojoyin:fix/set-cookies-from-dict
Open

Handle cookies dict set to prevent AttributeError#91
rojoyin wants to merge 1 commit intodaijro:mainfrom
rojoyin:fix/set-cookies-from-dict

Conversation

@rojoyin
Copy link

@rojoyin rojoyin commented Mar 12, 2025

Use merge_cookies instead of individual set_cookie calls when handling dictionary cookies. This ensures:

  • Consistent behavior across all cookie types (dict, list, RequestsCookieJar)
  • Better handling of cookie merging and potential conflicts
  • Proper domain and path handling through merge_cookies function

Open issue: #90

@rojoyin rojoyin changed the title Handle cookies dict correctly in Session.get() to prevent AttributeError Handle cookies dict correctly in Session.get() to prevent AttributeError Mar 12, 2025
@rojoyin rojoyin changed the title Handle cookies dict correctly in Session.get() to prevent AttributeError Handle cookies dict correctly in Session.get() to prevent AttributeError Mar 12, 2025
@rojoyin rojoyin changed the title Handle cookies dict correctly in Session.get() to prevent AttributeError Handle cookies dict set to prevent AttributeError Mar 12, 2025
@Miskler
Copy link

Miskler commented Aug 31, 2025

I was just passing by, not a maintainer, but I have a very important and useful opinion :) In your PR, Git detected a full file replacement. I think you should fix that before merging.

@rojoyin rojoyin force-pushed the fix/set-cookies-from-dict branch from 4584b7a to e88d7dd Compare September 1, 2025 08:43
@rojoyin
Copy link
Author

rojoyin commented Sep 1, 2025

I was just passing by, not a maintainer, but I have a very important and useful opinion :) In your PR, Git detected a full file replacement. I think you should fix that before merging.

Thanks for the feedback. Just fixed

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.

2 participants