fix: Remove tmp folder for PlaywrightCrawler in non-headless mode by Mantisus · Pull Request #1046 · apify/crawlee-python

Skip to content

Navigation Menu

Sign in

Appearance settings

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up

Appearance settings

Conversation

@Mantisus

Copy link

Collaborator

@Mantisus Mantisus commented

Mar 3, 2025

edited

Loading

Description

  • Fix a bug removing a temporary folder for non-headless mode. I saved an attempt to remove a folder on the close event, for when for some reason the browser crashes and the context is closed by the Playwright mechanisms

@Mantisus Mantisus requested review from Pijukatel and vdusek and removed request for vdusek

March 3, 2025 17:25

@Mantisus Mantisus self-assigned this

Mar 3, 2025
Copy link

Collaborator

@vdusek vdusek left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

Collaborator

@Pijukatel Pijukatel left a comment

Choose a reason for hiding this comment

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

Could you please add tiny test for this scenario that checks that the dir exists and is deleted when close is called.

@Mantisus Mantisus force-pushed the fix-remove-tmp-files branch 2 times, most recently from 83992a0 to 639c797 Compare

March 4, 2025 19:16

@Mantisus Mantisus force-pushed the fix-remove-tmp-files branch from 639c797 to 479dd4c Compare

March 5, 2025 00:52

@Pijukatel Pijukatel self-requested a review

March 5, 2025 07:32

@vdusek vdusek merged commit 3a7f444 into apify:master

Mar 6, 2025

23 checks passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vdusek vdusek vdusek approved these changes

@Pijukatel Pijukatel Pijukatel approved these changes

Assignees

@Mantisus Mantisus

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Mantisus @vdusek @Pijukatel