Screenshot scraper integration #558
Conversation
| browser.close() | ||
|
|
||
| for screenshot_data in screenshot_data_list: | ||
| screenshots.append(screenshot_data) |
There was a problem hiding this comment.
why having duplicate data structures screenshots and screenshot_data_list?
keep only one of the two, and drop the other
|
|
||
| capture_screenshot(0, screenshot_counter) | ||
| screenshot_counter += 1 | ||
| capture_screenshot(viewport_height, screenshot_counter) |
There was a problem hiding this comment.
infinite-scrolling web pages (with dynamic JS rendering) may have undefined behavior with the viewport height, as that is bound to change as well as you scroll through the page in the browser.
looks fine for now, but I would maybe add a disclaimer in the docs saying screenshot capturing might not work as well for those; collecting only two screenshots is fine as well, but that number might better be a function of the viewport height, so that we don't miss parts of the page content in case a page is very very long (although not infinite-scrolling)
|
|
||
| api_key = self.node_config.get("config", {}).get("llm", {}).get("api_key", "") | ||
|
|
||
| supported_models = ["gpt-4o", "gpt-4o-mini", "gpt-4-turbo"] |
There was a problem hiding this comment.
all open VLMs would work fine as well if run with ollama or similar, same goes for gemini-pro
why such a strict set of models?
P.S. - turn it into a set instead of a list
| } | ||
|
|
||
| payload = { | ||
| "model": "gpt-4o-mini", |
There was a problem hiding this comment.
why hardcoding the mini version if a support model was passed by the user?
| is not supported. Supported models are: | ||
| {', '.join(supported_models)}.""") | ||
|
|
||
| for image_data in images: |
There was a problem hiding this comment.
better off using the async API and processing the images in parallel so not to waste time for nothing
| """ | ||
| fetch_screen_node = FetchScreenNode( | ||
| input="url", | ||
| output=["imgs"], |
There was a problem hiding this comment.
why putting the key here if the node doesn't use it and hardcodes it as screenshots?
| ) | ||
| generate_answer_from_image_node = GenerateAnswerFromImageNode( | ||
| input="doc", | ||
| output=["parsed_doc"], |
There was a problem hiding this comment.
same goes here, neither the input nor output arguments are really used by the node
| } | ||
|
|
||
| return state | ||
| def execute(self, state: dict) -> dict: |
There was a problem hiding this comment.
wrap the function with a safer guard on the running event loop, e.g.:
def execute(self, state: dict) -> dict:
"""
Wrapper to run the asynchronous execute_async function in a synchronous context.
"""
try:
eventloop = asyncio.get_event_loop()
except RuntimeError:
eventloop = None
if eventloop and eventloop.is_running():
state = eventloop.run_until_complete(self.execute_async(state))
else:
state = asyncio.run(self.execute_async(state))
return state
|
🎉 This issue has been resolved in version 1.14.0-beta.13 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
|
🎉 This issue has been resolved in version 1.14.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
No description provided.