Skip to content

refactor!: change Session cookies from dict to SessionCookies with CookieJar#984

Merged
vdusek merged 20 commits into
apify:masterfrom
Mantisus:session-with-cookiejar
Feb 25, 2025
Merged

refactor!: change Session cookies from dict to SessionCookies with CookieJar#984
vdusek merged 20 commits into
apify:masterfrom
Mantisus:session-with-cookiejar

Conversation

@Mantisus

@Mantisus Mantisus commented Feb 13, 2025

Copy link
Copy Markdown
Collaborator

Description

  • Change the cookie storage in the Session from dict to SessionCookies which uses CookieJar. It supports basic cookie parameters and multiple domains.

Issues

Testing

  • Update old tests
  • Add tests for new class
  • Add tests for multidomain support
@Mantisus Mantisus self-assigned this Feb 13, 2025
@Mantisus Mantisus requested review from Pijukatel and vdusek February 13, 2025 12:51
@Mantisus Mantisus marked this pull request as ready for review February 13, 2025 12:51
Comment thread src/crawlee/crawlers/_playwright/_playwright_crawler.py Outdated

@vdusek vdusek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Since it contains a breaking change, could you please describe the breaking changes in the PR's description and also summarize it in the Upgrading guide?

Comment thread src/crawlee/crawlers/_playwright/_playwright_crawler.py Outdated
Comment thread src/crawlee/sessions/_cookies.py Outdated
@Mantisus Mantisus requested review from Pijukatel and vdusek February 20, 2025 11:32

@vdusek vdusek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There are failing tests for Windows Python 3.12.

@Mantisus Mantisus force-pushed the session-with-cookiejar branch from 003faf8 to f136bcd Compare February 20, 2025 23:22
@Mantisus

Copy link
Copy Markdown
Collaborator Author

There are failing tests for Windows Python 3.12.

The problem is not related to this PR.

It's solved in the PR #1007

Pijukatel
Pijukatel previously approved these changes Feb 21, 2025
@Mantisus Mantisus requested a review from vdusek February 21, 2025 13:46

@vdusek vdusek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good, just a few minor comments and one concern regarding the usage of HTTPX's cookies.

Comment thread docs/upgrading/upgrading_to_v0x.md Outdated
Comment thread src/crawlee/sessions/_cookies.py Outdated
Comment thread src/crawlee/crawlers/_playwright/_playwright_crawler.py Outdated
Comment thread src/crawlee/sessions/_cookies.py Outdated
Comment thread src/crawlee/sessions/_cookies.py Outdated
Comment thread src/crawlee/sessions/_cookies.py Outdated
@Mantisus Mantisus requested a review from vdusek February 25, 2025 01:35
Comment thread src/crawlee/sessions/_cookies.py Outdated
Comment thread src/crawlee/sessions/_cookies.py Outdated
@Pijukatel Pijukatel dismissed their stale review February 25, 2025 08:02

New commits with new comments

Comment thread src/crawlee/sessions/_cookies.py Outdated
Comment thread src/crawlee/sessions/_cookies.py
Comment thread src/crawlee/sessions/_cookies.py Outdated
Comment thread src/crawlee/sessions/_cookies.py Outdated
Comment thread src/crawlee/sessions/_cookies.py Outdated
Comment thread src/crawlee/sessions/_cookies.py Outdated
Comment thread src/crawlee/sessions/_cookies.py
Comment thread src/crawlee/sessions/_cookies.py
Comment thread src/crawlee/sessions/_cookies.py
Mantisus and others added 6 commits February 25, 2025 12:12
Co-authored-by: Vlada Dusek <v.dusek96@gmail.com>
Co-authored-by: Vlada Dusek <v.dusek96@gmail.com>
Co-authored-by: Vlada Dusek <v.dusek96@gmail.com>
Co-authored-by: Vlada Dusek <v.dusek96@gmail.com>
Co-authored-by: Vlada Dusek <v.dusek96@gmail.com>
@Pijukatel Pijukatel self-requested a review February 25, 2025 10:50

@vdusek vdusek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM and great job, just wait for the resolution of the last unresolved conversation

@vdusek vdusek merged commit 6523b3a into apify:master Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

5 participants