fix(docker-api): apply base_config to fields the client did not send - #2122
Open
bong-u wants to merge 1 commit into
Open
fix(docker-api): apply base_config to fields the client did not send#2122bong-u wants to merge 1 commit into
bong-u wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2121
crawler.base_confignever applies to a field whoseCrawlerRunConfigdefault isFalseor0.The guard from a1950af (#1505) reads "the client didn't send this" as "the loaded attribute is
Noneor""":simulate_userdefaults toFalse, so thesetattris skipped and the stock image has never honored its ownconfig.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:
This handles both payload shapes, the flat dict and
{"type": ..., "params": {...}}, without sniffing the type:CrawlerRunConfighas no field namedparams.Both call sites now share the helper instead of duplicating the guard.
Behavior change: the stock image now runs with
simulate_useron for the first time, so crawls get slower.That is what
config.ymlhas always asked for, but existing deployments will feel it.Happy to drop
simulate_userfrom 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 beforeCrawlerRunConfig.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 grepsapi.pyfor the literal"for cfg in config_list:", which becomesfor cfg, raw in zip(config_list, crawler_configs):, so I updated the string.config_listis a 1:1 comprehension overcrawler_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 passedAlso against a local Docker build with the stock
config.yml:POST /crawlwith{"urls": ["https://example.com"]}and nocrawler_configgiveseffective_config.simulate_user == Falsebefore the fix andTrueafter.Checklist: