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.

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

feat: [WIP] apify extra #116

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

feat: [WIP] apify extra #116

wants to merge 9 commits into from

Conversation

barjin
Copy link
Contributor

@barjin barjin commented Oct 28, 2022

No description provided.

@barjin barjin added the do not merge Under any circumstances, do not merge this PR! The author has not fully finished their work yet. label Oct 28, 2022
@barjin barjin changed the title [WIP] feat: apify extra feat: [WIP] apify extra Oct 28, 2022
@B4nan B4nan marked this pull request as draft November 7, 2022 12:45
@metalwarrior665
Copy link
Member

@barjin Are we still stuck on something or there wasn't the capacity to finalize? I will definitely want to polish the code once more but also would like to do it when we know how to build it :)

@barjin barjin removed the do not merge Under any circumstances, do not merge this PR! The author has not fully finished their work yet. label Nov 29, 2022
@barjin
Copy link
Contributor Author

barjin commented Nov 29, 2022

feel free to review, guess it sort of fizzled out @B4nan ?

btw it's been buildable since the first day, it's all kinda finished now - correctness and usability remain to be checked by someone with more actor-building expertise :)

@barjin barjin marked this pull request as ready for review November 30, 2022 09:32
@metalwarrior665
Copy link
Member

Hah, I didn't even realize you refactored the whole thing and added tests :) Let me just review it and we can release, good job!

@metalwarrior665
Copy link
Member

btw @barjin @B4nan I'm thinking about this one from time to time. Last year, there were only few new projects so there wasn't much need to implement any advanced functionality (and what we needed was fine to copy/paste).

In the end, we can probably just keep copy-pasting for now. If we want to push this through, I think it would be better to do it right into Crawlee, the interface to Dataset is the same and it will bubble into SDK.

The only other one that is used quite frequently is parallelPersistedCall (see example here) which should probably go to the SDK. The client would be cool but it doesn't understand migration which is needed in this case.

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