feat: Handle request list user input#326
Conversation
github-actions
Bot
added
t-tooling
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request is neither linked to an issue or epic nor labeled as adhoc!
Pijukatel
changed the title
Fix typing issues WIP
TODO: Finish test for it. WIP
Add test for checking all genrated request properties.
Add helper class for input keys Add top level Input class for handling actor inputs.
Add few more tests for regex
It had too many assumptions that users might not be interested in. Users should create such Input helper classes based on their specific inputs and their names.
Update TCH001, TCH002, TCH003 uses.
c237c66 to
6ff9e90
Compare
|
Sorry for the force-push. Due to my previous mainly Gerrit-based experience I was expecting this Rebase button in Github to do something more clean in the log... |
| inline-quotes = "single" | ||
|
|
||
| [tool.ruff.lint.flake8-type-checking] | ||
| runtime-evaluated-base-classes = ["pydantic.BaseModel", "crawlee.configuration.Configuration"] |
There was a problem hiding this comment.
Even though crawlee.configuration.Configuration inherits from pydantic_settings.BaseSettings and that one from pydantic.BaseModel, ruff has some issues in following inheritance hierarchy too far. So until that changes, some models will have to be explicitly mentioned even though they inherit from pydantic.BaseModel.
See closed issue where this is described as known limitation to certain degree:
astral-sh/ruff#8725
janbuchar
left a comment
janbuchar
left a comment
There was a problem hiding this comment.
This is looking pretty good! Just a bunch of nits to iron out...
| headers=request_input.headers, | ||
| user_data=request_input.user_data, | ||
| ) | ||
| for request_input in simple_url_inputs |
There was a problem hiding this comment.
Btw thinking about this class reminded me of this: apify/crawlee#2466
Basically, in JS Crawlee, we used to instantiate all the requests at the beginning (like here), but that ate up much more memory than just keeping the POJO objects (I guess dicts in Python) and instantiating the Request class only once we really need it. Maybe Python handles this better (has slimmer class instances).
It was at that time we learned that users love to make >1.000.000 requests long RLs 😅
There was a problem hiding this comment.
That is a valid point, but I think it will be done later in apify/crawlee-python#99 as the current class is openly not as mature as the JS version:
https://github.com/apify/crawlee-python/blob/master/src/crawlee/storages/_request_list.py#L23
There was a problem hiding this comment.
Agreed, let's make sure the SDK supports everything the platform can offer first, then we can optimize.
| url_input_adapter = TypeAdapter(list[Union[_RequestsFromUrlInput, _SimpleUrlInput]]) | ||
|
|
||
|
|
||
| # @docs_group('Classes') # Not yet available in crawlee |
| headers=request_input.headers, | ||
| user_data=request_input.user_data, | ||
| ) | ||
| for request_input in simple_url_inputs |
There was a problem hiding this comment.
Agreed, let's make sure the SDK supports everything the platform can offer first, then we can optimize.
Co-authored-by: Jan Buchar <jan.buchar@apify.com>
5 participants
Add helper function to handle request list inputs.
Closes: #310