fix(ci): skip sqlalchemy bigquery profiler - #17787
Conversation
ef7fa65 to
9bfa67f
Compare
There was a problem hiding this comment.
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)
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
|
We could remove the constraint in setup.py since this is the only package that has an upper bound on |
9bfa67f to
a7effa0
Compare
This PR adds a workaround to skip import profiling for
packages/sqlalchemy-bigquerywhen running on Python 3.15, as the package currently restricts Python version support to<3.15.Key Changes
.github/workflows/import-profiler.ymlto skippackages/sqlalchemy-bigqueryinside the modified package loop.