Skip to content

fix(profiler): prevent filtering target package name in module discovery - #18072

Open
hebaalazzeh wants to merge 3 commits into
mainfrom
fix/profiler-test-utils-module-resolution
Open

fix(profiler): prevent filtering target package name in module discovery#18072
hebaalazzeh wants to merge 3 commits into
mainfrom
fix/profiler-test-utils-module-resolution

Conversation

@hebaalazzeh

Copy link
Copy Markdown
Contributor

Fix module resolution for packages like google-cloud-testutils whose top-level Python package (test_utils) was being filtered out by ignored_prefixes in find_module_from_package.

@hebaalazzeh hebaalazzeh self-assigned this Aug 11, 2026
@hebaalazzeh
hebaalazzeh force-pushed the fix/profiler-test-utils-module-resolution branch from c608b53 to 7af75c7 Compare August 11, 2026 18:48

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

Copy link
Copy Markdown
Contributor

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 introduces PEP 0810 explicit lazy imports (lazy_modules) across several packages in google-cloud-core and updates the import profiler script to better handle namespace packages. The review feedback suggests placing the lazy_modules definitions before import statements in accordance with PEP 8, which would also eliminate the need for .flake8 E402 bypasses and typing imports. Additionally, the feedback points out a logic issue in the import profiler's package filtering that causes false positives for packages like google-cloud-build, recommending a more specific exception for google-cloud-testutils.

I am having trouble creating individual review comments. Click here to see my feedback.

scripts/import_profiler/profiler.py (413-424)

high

The current logic pkg_clean.endswith(top.replace("_", "")) introduces false positives for other packages whose names end with an ignored prefix.

For example:

  • For google-cloud-build, pkg_clean is "googlecloudbuild". If a standard "build" directory exists, top is "build". Since "googlecloudbuild" ends with "build", the "build" directory will NOT be ignored, which is incorrect as it is just a build artifact directory.
  • For google-cloud-notebooks, the "notebooks" directory (which contains Jupyter notebooks and is in ignored_prefixes) will NOT be ignored because "googlecloudnotebooks" ends with "notebooks".

Since google-cloud-testutils is the only package in the repository whose top-level Python package is actually in ignored_prefixes (test_utils), we should make this exception specific to google-cloud-testutils and test_utils. This avoids false positives for all other packages.

            ignored_prefixes = ("tests", "samples", "examples", "benchmark", "benchmarks", "third_party", "testing", "test_utils", "docs", "build", "dist", "bin", "ci", "scripts", "cloudbuild", "notebooks", "assets", "scratch", "specs")
            ignored_starts = ("test_", "sample_", "bench_", "example_", "doc_", "notebook_")
            
            filtered = []
            for p in pkgs:
                top = p.split(".")[0]
                is_ignored_top = top in ignored_prefixes or top.startswith(ignored_starts)
                if is_ignored_top and pkg == "google-cloud-testutils" and top == "test_utils":
                    is_ignored_top = False
                if is_ignored_top or p in ("google", "google.cloud"):
                    continue
                filtered.append(p)

@hebaalazzeh
hebaalazzeh marked this pull request as ready for review August 11, 2026 20:30
@hebaalazzeh
hebaalazzeh requested a review from a team as a code owner August 11, 2026 20:30
@hebaalazzeh
hebaalazzeh force-pushed the fix/profiler-test-utils-module-resolution branch from 7af75c7 to 4e3ba12 Compare August 11, 2026 21:03
Comment thread scripts/import_profiler/profiler.py Outdated
Comment thread scripts/import_profiler/profiler.py Outdated
Comment thread scripts/import_profiler/profiler.py
@hebaalazzeh
hebaalazzeh force-pushed the fix/profiler-test-utils-module-resolution branch from 24d9a30 to f24e53d Compare August 11, 2026 23:09
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.

2 participants