Skip to content

fix(docker-api): apply base_config to fields the client did not send - #2122

Open
bong-u wants to merge 1 commit into
unclecode:developfrom
bong-u:bugfix/base-config-boolean
Open

fix(docker-api): apply base_config to fields the client did not send#2122
bong-u wants to merge 1 commit into
unclecode:developfrom
bong-u:bugfix/base-config-boolean

Conversation

@bong-u

@bong-u bong-u commented Aug 5, 2026

Copy link
Copy Markdown

Summary

Fixes #2121

crawler.base_config never applies to a field whose CrawlerRunConfig default is False or 0.
The guard from a1950af (#1505) reads "the client didn't send this" as "the loaded attribute is None or """:

current_value = getattr(crawler_config, key)
if current_value is None or current_value == "":   # api.py:715
    setattr(crawler_config, key, value)

simulate_user defaults to False, so the setattr is skipped and the stock image has never honored its own config.yml.
Same for magic, override_navigator, check_robots_txt, remove_overlay_elements, page_timeout.

Dropping the guard would bring #1505 back.
After CrawlerRunConfig.load() nothing tells "omitted" apart from "sent, equal to the default".
Only the raw request dict still knows, so the fix reads the keys the client actually sent:

def _apply_base_config(cfg, base_config: dict, raw: Optional[dict]) -> None:
    provided = set(raw.get("params", raw)) if raw else set()
    for key, value in base_config.items():
        if key not in provided and hasattr(cfg, key):
            setattr(cfg, key, value)

This handles both payload shapes, the flat dict and {"type": ..., "params": {...}}, without sniffing the type: CrawlerRunConfig has no field named params.
Both call sites now share the helper instead of duplicating the guard.

Behavior change: the stock image now runs with simulate_user on for the first time, so crawls get slower.
That is what config.yml has always asked for, but existing deployments will feel it.
Happy to drop simulate_user from the shipped default config in this PR if you'd rather it land with no visible change.

List of files changed and why

  • deploy/docker/api.py: the fix. Adds _apply_base_config(), captures the raw dict before CrawlerRunConfig.load() overwrites the name, and replaces both copies of the guard.
  • tests/test_base_config_defaults.py (new): a regression test for the boolean case plus a non-regression test for [Bug]: Docker server base_config overwrites user CrawlerRunConfig settings despite user explicitly setting values #1505 (client value still wins), covering both payload shapes.
    No browser or server needed.
  • tests/test_issue_1837_config_list.py: it greps api.py for the literal "for cfg in config_list:", which becomes for cfg, raw in zip(config_list, crawler_configs):, so I updated the string.
    config_list is a 1:1 comprehension over crawler_configs, so the zip is safe.

How Has This Been Tested?

pytest tests/test_base_config_defaults.py tests/test_issue_1837_config_list.py
# 16 passed

Also against a local Docker build with the stock config.yml: POST /crawl with {"urls": ["https://example.com"]} and no crawler_config gives effective_config.simulate_user == False before the fix and True after.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added/updated unit tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

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.

1 participant