feat: Request Queue v2 #1975

vladfrangu · 2023-07-12T15:28:01Z

B4nan

left a few initial comments. it would be good to use a common predecessor, as now it's hard to see what actually changed in a 1000 LoC file that is mostly a copy of the v1 approach.

packages/core/src/storages/request_queue_v2.ts

packages/basic-crawler/src/internals/basic-crawler.ts

packages/puppeteer-crawler/src/internals/enqueue-links/click-elements.ts

packages/core/src/storages/request_queue_v2.ts

B4nan · 2023-08-08T14:54:27Z

@drobnikj can you please take a look as well

@vladfrangu I also see the puppeteer e2e tests failed on platform due to a chrome version mismatch, any idea why is that? maybe its just flakyness (since the scheduled run passed just fine), but it feels pretty weird

B4nan · 2023-08-08T14:55:46Z

We will also surely need some unit tests for this, I don't want you to rewrite all existing tests to support v2, but with e2e tests we won't have such a confidence (as they won't run on each PR).

drobnikj

Wow, that is a lot of code, looks good as a good start, on top of the comments I miss these parts from spec.:

If the actor is migrating(??aborting/exiting/failing), it should unlock all requests which locked
Ensuring that the queue is empty should wait for the lockSecs after it is empty to potential request lock can be expired.

packages/basic-crawler/src/internals/basic-crawler.ts

packages/core/src/storages/request_queue_v2.ts

packages/memory-storage/src/resource-clients/request-queue.ts

vladfrangu · 2023-08-25T11:56:45Z

Ready for another pair of reviews! Still need to add some tests for this, but I'm more confident it should just work

packages/basic-crawler/src/internals/basic-crawler.ts

drobnikj

The structure of the code is much easier to read 👍

I'm still not sure if the local cache would not mess queue in parallel execution, can we do a simple test where we run multiple scrapers on one large site with shared request queue?

packages/core/src/request.ts

packages/core/src/storages/request_provider.ts

packages/core/src/storages/request_queue_v2.ts

github-actions

⚠️ Pull Request Tookit has failed!

Pull request is neither linked to an issue or epic nor labeled as adhoc!

github-actions

⚠️ Pull Request Tookit has failed!

Pull request is neither linked to an issue or epic nor labeled as adhoc!

vladfrangu · 2023-09-18T13:05:32Z

~~E2E running: https://github.com/apify/crawlee/actions/runs/6222942549~~

~~E2E re-running: https://github.com/apify/crawlee/actions/runs/6223756273~~

~~E2E re-re-running: https://github.com/apify/crawlee/actions/runs/6223967154~~

E2E re-re-re-running: https://github.com/apify/crawlee/actions/runs/6224902038

github-actions

⚠️ Pull Request Tookit has failed!

Pull request is neither linked to an issue or epic nor labeled as adhoc!

github-actions

⚠️ Pull Request Tookit has failed!

Pull request is neither linked to an issue or epic nor labeled as adhoc!

B4nan · 2023-09-19T10:35:07Z

FYI for the PR toolkit to pass, every PR needs to have an estimate and a correct label (or be connected to an issue that has those - the link needs to be set via zenhub, its unfortunately not enough to provide the closes #123 part, at least not after the PR is created).

github-actions

⚠️ Pull Request Tookit has failed!

Pull request is neither linked to an issue or epic nor labeled as adhoc!

packages/basic-crawler/src/internals/basic-crawler.ts

packages/core/src/storages/request_provider.ts

Co-authored-by: Martin Adámek <banan23@gmail.com>

drobnikj

Just nits, I would like to test it once the testing version will be publish npm

packages/basic-crawler/src/internals/basic-crawler.ts

packages/core/src/storages/request_provider.ts

drobnikj

Just nits, I would like to test it once the testing version will be publish npm

B4nan

I guess this is good to go?

barjin

End of an era, really 😄

Looks good to me, let's see how it runs

Strajk · 2023-09-21T09:58:07Z

vladfrangu mentioned this pull request Jul 21, 2023

Request queue v2 - locking #1365

Closed

vladfrangu force-pushed the feat/request-queue-v2 branch from 5bb58a9 to 3bc18db Compare August 8, 2023 13:22

vladfrangu requested a review from B4nan August 8, 2023 13:31

B4nan reviewed Aug 8, 2023

View reviewed changes

B4nan requested review from drobnikj and barjin August 8, 2023 14:54

drobnikj requested changes Aug 9, 2023

View reviewed changes

vladfrangu requested review from drobnikj and B4nan August 25, 2023 11:48

vladfrangu force-pushed the feat/request-queue-v2 branch from fc0de1d to a1b3e95 Compare August 25, 2023 11:50

B4nan reviewed Aug 25, 2023

View reviewed changes

packages/basic-crawler/src/internals/basic-crawler.ts Outdated Show resolved Hide resolved

vladfrangu force-pushed the feat/request-queue-v2 branch from 18d4395 to 415b0f5 Compare August 25, 2023 12:49

vladfrangu requested a review from B4nan August 25, 2023 13:59

drobnikj requested changes Aug 29, 2023

View reviewed changes

vladfrangu force-pushed the feat/request-queue-v2 branch from c2fd6ec to 08cceb5 Compare September 11, 2023 08:29

vladfrangu marked this pull request as ready for review September 11, 2023 08:41

github-actions bot assigned vladfrangu Sep 14, 2023

github-actions bot added this to the 72nd sprint - Tooling team milestone Sep 14, 2023

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 Sep 14, 2023

github-actions bot reviewed Sep 14, 2023

View reviewed changes

vladfrangu requested a review from drobnikj September 14, 2023 11:27

github-actions bot reviewed Sep 18, 2023

View reviewed changes

vladfrangu added 3 commits September 18, 2023 17:28

chore: some groundwork, need to be less zombie to continue

22e4a2c

chore: more handling for queue hydration

36de203

chore: finish getOrHydrateRequest 🤞

bb6e2b9

vladfrangu added 12 commits September 18, 2023 17:28

chore: fix test

62b75ee

chore: fix bugs, implement support for abort/migrating dropping locks

e9aa52f

chore: small inconsistencies fixed

f1df5fb

chore: fix build

fc654d3

chore: revert back to inProgress

5a09526

chore: limit list and lock head to 25 too

078eaaa

chore: more requested changs

0d82097

chore: lint

b83876f

chore: remove strict validation of unknown fields in Request

7d6efdc

chore: use internal timeouts instead

f77ab72

chore: RQv2 tests

e49d90e

chore: bump storage local to test in e2e too

a8879f6

vladfrangu force-pushed the feat/request-queue-v2 branch from 9f45080 to a8879f6 Compare September 18, 2023 14:28

github-actions bot reviewed Sep 18, 2023

View reviewed changes

chore: rebase woes

ffe5f51

github-actions bot reviewed Sep 18, 2023

View reviewed changes

B4nan added the tech roadmap Issues contributing to tech roadmap. label Sep 19, 2023

github-actions bot reviewed Sep 19, 2023

View reviewed changes

B4nan reviewed Sep 19, 2023

View reviewed changes

packages/basic-crawler/src/internals/basic-crawler.ts Outdated Show resolved Hide resolved

packages/basic-crawler/src/internals/basic-crawler.ts Outdated Show resolved Hide resolved

packages/core/src/storages/request_provider.ts Outdated Show resolved Hide resolved

vladfrangu and others added 2 commits September 19, 2023 18:24

Update packages/basic-crawler/src/internals/basic-crawler.ts

60e5653

Co-authored-by: Martin Adámek <banan23@gmail.com>

chore: requested changes

083337c

drobnikj requested changes Sep 20, 2023

View reviewed changes

packages/basic-crawler/src/internals/basic-crawler.ts Show resolved Hide resolved

packages/core/src/storages/request_provider.ts Show resolved Hide resolved

drobnikj requested changes Sep 20, 2023

View reviewed changes

fix: incorrectly overriding methods

bf11fb5

B4nan approved these changes Sep 21, 2023

View reviewed changes

barjin approved these changes Sep 21, 2023

View reviewed changes

B4nan merged commit 70a77ee into master Sep 21, 2023
10 of 11 checks passed

B4nan deleted the feat/request-queue-v2 branch September 21, 2023 10:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Request Queue v2 #1975

feat: Request Queue v2 #1975

vladfrangu commented Jul 12, 2023 •

edited by B4nan

B4nan left a comment

B4nan commented Aug 8, 2023

B4nan commented Aug 8, 2023

drobnikj left a comment

vladfrangu commented Aug 25, 2023

drobnikj left a comment

github-actions bot left a comment

github-actions bot left a comment

vladfrangu commented Sep 18, 2023 •

edited

github-actions bot left a comment

github-actions bot left a comment

B4nan commented Sep 19, 2023 •

edited

github-actions bot left a comment

drobnikj left a comment

drobnikj left a comment

B4nan left a comment

barjin left a comment

Strajk commented Sep 21, 2023

feat: Request Queue v2 #1975

feat: Request Queue v2 #1975

Conversation

vladfrangu commented Jul 12, 2023 • edited by B4nan

B4nan left a comment

Choose a reason for hiding this comment

B4nan commented Aug 8, 2023

B4nan commented Aug 8, 2023

drobnikj left a comment

Choose a reason for hiding this comment

vladfrangu commented Aug 25, 2023

drobnikj left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

vladfrangu commented Sep 18, 2023 • edited

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

B4nan commented Sep 19, 2023 • edited

github-actions bot left a comment

Choose a reason for hiding this comment

drobnikj left a comment

Choose a reason for hiding this comment

drobnikj left a comment

Choose a reason for hiding this comment

B4nan left a comment

Choose a reason for hiding this comment

barjin left a comment

Choose a reason for hiding this comment

Strajk commented Sep 21, 2023

vladfrangu commented Jul 12, 2023 •

edited by B4nan

vladfrangu commented Sep 18, 2023 •

edited

B4nan commented Sep 19, 2023 •

edited