[python-SDK] improvs/async #1337
Conversation
tomkosm
left a comment
tomkosm
left a comment
There was a problem hiding this comment.
Looks good overall, commented on a few minor mostly type related stuff
| blockAds: Optional[bool] = None | ||
| proxy: Optional[Literal["basic", "stealth"]] = None | ||
|
|
||
| class Action(pydantic.BaseModel): |
There was a problem hiding this comment.
Maybe we should make a different type for each action, so its clear what the arguments for each action are?
There was a problem hiding this comment.
You then can make action be a union ie Union[WaitAction, ClickAction, ScreenshotAction]
There was a problem hiding this comment.
good idea!
| """ | ||
| crawl_response = self.async_crawl_url(url, params, idempotency_key) | ||
| if crawl_response['success'] and 'id' in crawl_response: | ||
| return CrawlWatcher(crawl_response['id'], self) | ||
| else: | ||
| raise Exception("Crawl job failed to start") | ||
|
|
||
| def map_url(self, url: str, params: Optional[Dict[str, Any]] = None) -> Any: | ||
| def map_url(self, url: str, params: Optional[Dict[str, Any]] = None) -> MapResponse: |
There was a problem hiding this comment.
Same here, maybe we should use the type for params
| await asyncio.sleep(backoff_factor * (2 ** attempt)) | ||
| continue | ||
| if response.status != 200: | ||
| await self._handle_error(response, "make POST request") |
There was a problem hiding this comment.
this is a retry for 502s
| for attempt in range(retries): | ||
| try: | ||
| async with session.post(url, headers=headers, json=data) as response: | ||
| if response.status == 502: |
There was a problem hiding this comment.
nope. It's retry
| for attempt in range(retries): | ||
| try: | ||
| async with session.get(url, headers=headers) as response: | ||
| if response.status == 502: |
There was a problem hiding this comment.
we have this twice, maybe abstract it
There was a problem hiding this comment.
first is get, second is post
| else: | ||
| raise Exception(f'Failed to start crawl. Error: {response.get("error")}') | ||
|
|
||
| async def async_crawl_url(self, url: str, params: Optional[Dict[str, Any]] = None, idempotency_key: Optional[str] = None) -> CrawlResponse: |
| else: | ||
| raise Exception(f'Job failed or was stopped. Status: {status_data["status"]}') | ||
|
|
||
| async def map_url(self, url: str, params: Optional[Dict[str, Any]] = None) -> MapResponse: |
|
Add |
done |
|
@nickscamara can you take a look and publish this one pls? |
| this.init(); | ||
| } | ||
|
|
There was a problem hiding this comment.
The async initialization in this.init() is not awaited, which might lead to requests using a default version before the SDK version is properly set. Consider ensuring that initialization is complete (for example, by awaiting the promise) before processing further operations that depend on the version.
| this.init(); | |
| } | |
| } | |
| static async create(config: FirecrawlAppConfig): Promise<FirecrawlApp> { | |
| const instance = new FirecrawlApp(config); | |
| await instance.init(); | |
| return instance; | |
| } |
No description provided.