Keep request futures local to sherlock#2946
Open
jameswjr wants to merge 1 commit into
Open
Conversation
Member
|
Disregard the docker build failure, that's unrelated to your PR. |
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
This keeps request futures local to
sherlock()instead of storing them inside the caller-providedsite_datadictionaries.Rationale
Although Sherlock is primarily used as a CLI,
sherlock()is an importable function and is already called directly in the test suite. A wrapper, integration, or downstream test can reasonably pass ina
site_dataobject and expect it to remain site metadata rather than become temporary request-state storage.Currently, the function iterates through
site_datalike this:Inside that loop, net_info is the per-site dictionary from site_data. The current implementation stores runtime request state there:
net_info["request_future"] = futureand reads it back later.
This is harmless in the normal one-shot CLI path, because the mutated site_data is usually discarded. But it can surprise callers that reuse, inspect, compare, serialize, or share the same site_data
object.
This PR preserves the existing two-pass flow, but stores futures in a local dictionary keyed by social-network name instead.
Behavior
This should preserve user-visible behavior. It changes only where temporary request futures are stored during the call.
The invalid-username path is still handled: when no future is created, the site already has a status, and the second pass continues before reading from the local futures dictionary.
Test
Added a focused no-network regression test that fakes the futures session and asserts that sherlock() does not mutate the supplied site_data.
Local validation
python3 -m py_compile sherlock_project/sherlock.py tests/test_ux.pyI prepared this outside a fully installed Sherlock development environment, so I was not able to run the focused pytest command locally. The intended focused test command is:
python -m pytest -q tests/test_ux.py::test_sherlock_does_not_mutate_input_site_data