Skip to content

Allow users to run crawls with 1 or 2 browser windows #2627

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

Merged
merged 57 commits into from
Jun 3, 2025

Conversation

tw4l
Copy link
Member

@tw4l tw4l commented May 27, 2025

Fixes #2425

Changed

  • Switch backend to primarily using number of browser windows rather than scale multiplier (including migration to calculate browserWindows from scale for existing workflows and crawls)
  • Still support scale in addition to browserWindows in input models for creating and updating workflows and re-adjusting live crawl scale for backwards compatibility
  • Adds new max_browser_windows value to Helm chart, but calculates the value from max_crawl_scale as fallback for users with that value already set in local charts
  • Rework frontend to allow users to select multiples of crawler_browser_instances or any value below crawler_browser_instances for browser windows. For instance, with crawler_browser_instances=4 and max_browser_windows=8, the user would be presented with the following options: 1, 2, 3, 4, 8
  • Sets maximum width of screencast to image width returned by message

@SuaYoo
Copy link
Member

SuaYoo commented May 29, 2025

@tw4l Updated to set the maximum width to the screencast image width:

Screenshot 2025-05-28 at 7 06 44 PM

@SuaYoo SuaYoo force-pushed the issue-2425-browser-windows branch from 8cb422e to aab9f5a Compare May 29, 2025 03:06
@ikreymer ikreymer marked this pull request as ready for review May 29, 2025 03:08
@tw4l tw4l requested review from ikreymer and SuaYoo May 29, 2025 03:45
@SuaYoo SuaYoo requested a review from emma-sg May 29, 2025 04:50
ikreymer and others added 2 commits May 29, 2025 11:33
- rename pod_count -> scale for consistency
- remove debug logging
- simplify update_scale to remove cast
Co-authored-by: Tessa Walsh <tessa@bitarchivist.net>
Copy link
Member

@SuaYoo SuaYoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good testing locally, if it's easy to do we should add a deprecation notice for the scale API field.

Copy link
Member

@ikreymer ikreymer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Nice work, tested on dev with different number of windows!

@ikreymer ikreymer merged commit dc41468 into main Jun 3, 2025
27 checks passed
@ikreymer ikreymer deleted the issue-2425-browser-windows branch June 3, 2025 20:37
tw4l pushed a commit that referenced this pull request Jun 12, 2025
)

- follow up to #2627 
- use qa_num_browser_windows to set exact number of QA browsers,
fallback to qa_scale
- set num_browser_windows and num_browsers_per_pod using crawler / qa
values depending if QA crawl
- scale_from_browser_windows() accepts optional browsers_per_pod if
dealing with possible QA override
- store 'desiredScale' in CrawlStatus to avoid recomputing for later
scale resolving
- ensure status.scale is always the actual scale observed
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.

[Feature]: Ability to use fewer than 4 browser windows
3 participants