Skip to content

fix: import urllib.parse explicitly instead of relying on side effects - #2804

Open
skippdot wants to merge 1 commit into
googleapis:mainfrom
skippdot:fix-urllib-parse-import
Open

fix: import urllib.parse explicitly instead of relying on side effects#2804
skippdot wants to merge 1 commit into
googleapis:mainfrom
skippdot:fix-urllib-parse-import

Conversation

@skippdot

@skippdot skippdot commented Sep 1, 2026

Copy link
Copy Markdown

Fixes #2803

Four modules import the bare urllib package but call urllib.parse.*: _helpers.py (4 call sites), discovery.py (6), http.py (6), model.py (1). import urllib does not import submodules — urllib.parse only becomes an attribute of the package after some module executes import urllib.parse. Today that happens as a side effect of httplib2 being imported transitively, which is why the bug is invisible in most runs.

_helpers.py has no third-party imports, so it crashes outright:

import googleapiclient._helpers as h
h._add_query_parameter("http://example.com", "a", "b")
# AttributeError: module 'urllib' has no attribute 'parse'

The same mechanism is why pytest tests/test__helpers.py in isolation fails 7 of 10 tests on main while the full-suite run passes.

Change

Replace import urllib with import urllib.parse in the four modules. Call sites are untouched — urllib.parse.urlparse(...) keeps its current spelling — so the diff is one line per module, +4/−4. The other three modules are included because they carry the same undeclared dependency on httplib2's import side effects, just without a currently-reachable crash.

Verification (Python 3.10 and 3.14)

  • The two-line reproducer above: crashed before, returns http://example.com?a=b after.
  • Fresh-subprocess import-and-use probe of each of the four modules: all OK.
  • pytest tests/test__helpers.py in isolation: 7 failed / 3 passed → 10 passed.
  • Full tests/ folder: 338 passed, 1 skipped on both Python versions.
  • black --check clean; flake8 --select=E9,F63,F7,F82 (the set CI enforces) clean.

- Replace `import urllib` with `import urllib.parse` in _helpers.py,
  discovery.py, http.py and model.py, which all call `urllib.parse.*`
- `import urllib` does not import submodules; the attribute only exists once
  some other module has imported urllib.parse, so _helpers crashed with
  AttributeError when used before any such import, and the other three
  modules depended on httplib2 importing it transitively
- Call sites are unchanged; this also fixes the 7 AttributeError failures
  when running tests/test__helpers.py in isolation

Fixes googleapis#2803
@skippdot
skippdot requested a review from a team as a code owner September 1, 2026 19:40
@product-auto-label product-auto-label Bot added the size: xs Pull request size is extra small. label Sep 1, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates several files within the googleapiclient library to import urllib.parse directly instead of the top-level urllib module. There are no review comments, and I have no feedback to provide.

@daniel-sanche daniel-sanche left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@daniel-sanche daniel-sanche added kokoro:force-run Add this label to force Kokoro to re-run the tests. kokoro:run Add this label to force Kokoro to re-run the tests. labels Sep 1, 2026
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: xs Pull request size is extra small.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

googleapiclient modules use urllib.parse but only import urllib, crashing when urllib.parse was not imported elsewhere

3 participants