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.

Sign up for GitHub

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

Jump to bottom

feat: implement global storage access checking and use it to prevent unwanted side effects in adaptive crawler #2371

Merged
merged 6 commits into from Feb 29, 2024
Merged

feat: implement global storage access checking and use it to prevent unwanted side effects in adaptive crawler #2371

merged 6 commits into from Feb 29, 2024

Conversation

janbuchar
Copy link
Contributor

@janbuchar janbuchar requested a review from B4nan February 28, 2024 15:39
@github-actions github-actions bot added this to the 84th sprint - Tooling team milestone Feb 28, 2024
@github-actions github-actions bot added t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics. labels Feb 28, 2024
B4nan
B4nan approved these changes Feb 28, 2024
View reviewed changes
Copy link
Member

@B4nan B4nan left a comment •
edited

Choose a reason for hiding this comment

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

looking good, just a few nits

(btw also ran the e2e tests on top of this and they are passing)

packages/core/src/storages/access_checking.ts Show resolved Hide resolved
packages/core/src/storages/request_queue.ts Outdated Show resolved Hide resolved
packages/playwright-crawler/src/internals/adaptive-playwright-crawler.ts Outdated Show resolved Hide resolved
packages/playwright-crawler/src/internals/adaptive-playwright-crawler.ts Outdated Show resolved Hide resolved
packages/playwright-crawler/src/internals/adaptive-playwright-crawler.ts Outdated Show resolved Hide resolved
@janbuchar @B4nan
Update packages/core/src/storages/access_checking.ts
af22737 
Co-authored-by: Martin Adámek <banan23@gmail.com>
@janbuchar janbuchar changed the title feat: implement global storage access checking and disable it in adaptive crawler feat: implement global storage access checking and use it to prevent unwanted side effects in adaptive crawler Feb 29, 2024
@janbuchar janbuchar merged commit fb3b7da into master Feb 29, 2024
8 checks passed
@janbuchar janbuchar deleted the checked-storage-access branch February 29, 2024 12:15
@B4nan B4nan mentioned this pull request Apr 21, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@B4nan B4nan B4nan approved these changes

Assignees

@janbuchar janbuchar

Labels
t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.
Projects
None yet
Milestone
84th sprint - Tooling team
Development

Successfully merging this pull request may close these issues.

Detect when user calls global SDK methods from "side-effect free" context
2 participants
@janbuchar @B4nan