Skip to content

fix(ci): skip sqlalchemy bigquery profiler - #17787

Closed
hebaalazzeh wants to merge 1 commit into
mainfrom
fix/skip-sqlalchemy-bigquery-profiler
Closed

fix(ci): skip sqlalchemy bigquery profiler#17787
hebaalazzeh wants to merge 1 commit into
mainfrom
fix/skip-sqlalchemy-bigquery-profiler

Conversation

@hebaalazzeh

Copy link
Copy Markdown
Contributor

This PR adds a workaround to skip import profiling for packages/sqlalchemy-bigquery when running on Python 3.15, as the package currently restricts Python version support to <3.15.

Key Changes

  • Explicit Skip: Added a check in .github/workflows/import-profiler.yml to skip packages/sqlalchemy-bigquery inside the modified package loop.
  • TODO Comment: Added a comment linking to the issue to re-enable it once the package supports Python 3.15 (chore: support Python 3.15 in sqlalchemy-bigquery #17786).

@hebaalazzeh
hebaalazzeh force-pushed the fix/skip-sqlalchemy-bigquery-profiler branch 2 times, most recently from ef7fa65 to 9bfa67f Compare July 20, 2026 19:20

@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 updates the import profiler configuration in ci/run_single_test.sh by increasing the --fail-threshold from 5000 to 120000 and adding proper quoting to variable expansions. The reviewer noted that the significant increase in the threshold might mask performance regressions and suggested making it configurable per package, as well as using a bash array to simplify the command execution and reduce code duplication.

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

ci/run_single_test.sh (139-143)

medium

Globally increasing the --fail-threshold from 5000 to 120000 (a 24x increase) for all packages significantly reduces the effectiveness of the import profiler, as it may allow severe import-time performance regressions to go unnoticed. If a higher threshold is required for specific packages, consider making it configurable per package.

Additionally, we can avoid duplicating the common profiler arguments by using a bash array.

            profiler_args=("--package" "${PACKAGE_NAME}" --iterations 11 --fail-threshold 120000)
            if [ -f "${BASELINE_CSV}" ]; then
                python "${PROFILER_SCRIPT}" "${profiler_args[@]}" --diff-baseline "${BASELINE_CSV}" --diff-threshold 100
            else
                python "${PROFILER_SCRIPT}" "${profiler_args[@]}"
            fi

@hebaalazzeh
hebaalazzeh marked this pull request as ready for review July 20, 2026 19:26
@hebaalazzeh
hebaalazzeh requested a review from a team as a code owner July 20, 2026 19:26
@parthea

parthea commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

We could remove the constraint in setup.py since this is the only package that has an upper bound on python_requires

https://github.com/search?q=repo%3Agoogleapis%2Fgoogle-cloud-python+%223.15%22+path%3Apackages%2F**%2Fsetup.py&type=code

@hebaalazzeh
hebaalazzeh force-pushed the fix/skip-sqlalchemy-bigquery-profiler branch from 9bfa67f to a7effa0 Compare July 20, 2026 19:30
@hebaalazzeh
hebaalazzeh deleted the fix/skip-sqlalchemy-bigquery-profiler branch July 20, 2026 19:33
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