-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Fix PreCommit YAML Xlang job hanging #37685
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
f9361c6
89b4b5e
dbda4f4
9709643
d03ed05
7c0a8d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3121,10 +3121,11 @@ class BeamModulePlugin implements Plugin<Project> { | |
| // TODO: https://github.com/apache/beam/issues/29022 | ||
| // pip 23.3 is failing due to Hash mismatch between expected SHA of the packaged and actual SHA. | ||
| // until it is resolved on pip's side, don't use pip's cache. | ||
| // pip 25.1 casues :sdks:python:installGcpTest stuck. Pin to 25.0.1 for now. | ||
| // Use pip 26.0.1 for improved resolution performance and bug fixes. See #34798. | ||
| args '-c', ". ${project.ext.envdir}/bin/activate && " + | ||
| "pip install --pre --retries 10 --upgrade pip==25.0.1 --no-cache-dir && " + | ||
| "pip install --pre --retries 10 --upgrade tox --no-cache-dir" | ||
| "pip install --pre --retries 10 --upgrade pip==26.0.1 --no-cache-dir && " + | ||
| "pip install --pre --retries 10 --upgrade tox --no-cache-dir && " + | ||
| "pip install uv" | ||
| } | ||
| } | ||
| // Gradle will delete outputs whenever it thinks they are stale. Putting a | ||
|
|
@@ -3169,9 +3170,19 @@ class BeamModulePlugin implements Plugin<Project> { | |
| packages += ",${extra}" | ||
| } | ||
|
|
||
| def pythonSdkDir = project.project(":sdks:python").projectDir | ||
| def constraintsPath = "${pythonSdkDir}/constraints.txt" | ||
| def constraintFile = project.file(constraintsPath) | ||
| def constraintFlag = constraintFile.exists() ? "--constraint ${constraintsPath}" : "" | ||
|
|
||
| // Use uv instead of pip - pip was hitting resolution-too-deep on tensorflow->keras->namex/optree. | ||
| // Include namex/optree as explicit deps to constrain resolution. | ||
| // --prerelease allow: envoy-data-plane depends on betterproto==2.0.0b6 (beta). | ||
| def anchorPkgs = "namex==0.0.9 optree==0.16.0" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think these deps are specific to a certain test suite, and thus configuring them is a matter of the test suite that needs them; BeamModulePlugin should concern with global configuration matters for the project; adding these deps doesn't feel like the right direction to me |
||
| def installCmd = ". ${project.ext.envdir}/bin/activate && uv pip install --prerelease allow ${constraintFlag} ${anchorPkgs} ${distTarBall}[${packages}]".replaceAll(/ +/, ' ').trim() | ||
| project.exec { | ||
| executable 'sh' | ||
| args '-c', ". ${project.ext.envdir}/bin/activate && pip install --pre --retries 10 ${distTarBall}[${packages}]" | ||
| args '-c', installCmd | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| # | ||
| # Licensed to the Apache Software Foundation (ASF) under one or more | ||
| # contributor license agreements. See the NOTICE file distributed with | ||
| # this work for additional information regarding copyright ownership. | ||
| # The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| # (the "License"); you may not use this file except in compliance with | ||
| # the License. You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| # | ||
|
|
||
| # Pins for installGcpTest. numpy handled via setup.py python_version markers. | ||
| # Use google-api-core 2.29.0 for py313 compat (2.16.2 lacks it). | ||
| googleapis-common-protos==1.72.0 | ||
| grpc-google-iam-v1>=0.12.4,<1.0.0 | ||
| google-api-core==2.29.0 | ||
| optree==0.16.0 | ||
| namex==0.0.9 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,8 +26,9 @@ requires = [ | |
| "mypy-protobuf==3.5.0", | ||
| # Avoid https://github.com/pypa/virtualenv/issues/2006 | ||
| "distlib==0.3.9", | ||
| # Numpy headers | ||
| "numpy>=1.14.3,<2.5.0", # Update setup.py as well. | ||
| # Numpy headers. py313 requires 2.1+; py<3.13 use 1.x for pandas ABI compat. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't follow why we need to cap numpy to
|
||
| "numpy>=1.26.0,<2.0.0; python_version < '3.13'", | ||
| "numpy>=2.1.0; python_version >= '3.13'", | ||
| # having cython here will create wheels that are platform dependent. | ||
| "cython>=3.0,<4", | ||
| ## deps for generating external transform wrappers: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -384,9 +384,10 @@ def get_portability_package_data(): | |
| 'grpcio>=1.67.0; python_version >= "3.13"', | ||
| 'httplib2>=0.8,<0.32.0', | ||
| 'jsonpickle>=3.0.0,<4.0.0', | ||
| # numpy can have breaking changes in minor versions. | ||
| # Use a strict upper bound. | ||
| 'numpy>=1.14.3,<2.5.0', # Update pyproject.toml as well. | ||
| # numpy: py310-312 use 1.x; py313 needs 2.x (1.x unsupported; avoids # pylint: disable=line-too-long | ||
| # pandas ABI mismatch). | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are there some detaills on this mismatch? |
||
| 'numpy>=1.26.0,<2.0.0; python_version < "3.13"', | ||
| 'numpy>=2.1.0; python_version >= "3.13"', | ||
| 'objsize>=0.6.1,<0.8.0', | ||
| 'packaging>=22.0', | ||
| 'pillow', | ||
|
|
@@ -541,6 +542,9 @@ def get_portability_package_data(): | |
| # tensorflow-transform requires dill, but doesn't set dill as a | ||
| # hard requirement in setup.py. | ||
| 'dill', | ||
| # namex/optree: pin to avoid resolver issues (lack version bounds) # pylint: disable=line-too-long | ||
| 'namex==0.0.9', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not an opinion, just wondering - should this be defined in ml_base instead? will |
||
| 'optree==0.16.0', | ||
| 'tensorflow-transform', | ||
| # Comment out xgboost as it is breaking presubmit python ml | ||
| # tests due to tag check introduced since pip 24.2 | ||
|
|
@@ -549,8 +553,13 @@ def get_portability_package_data(): | |
| ] + ml_base, | ||
| 'p312_ml_test': [ | ||
| 'datatable', | ||
| 'namex==0.0.9', | ||
| 'optree==0.16.0', | ||
| ] + ml_base, | ||
| 'p313_ml_test': [ | ||
| 'namex==0.0.9', | ||
| 'optree==0.16.0', | ||
| ] + ml_base, | ||
| 'p313_ml_test': ml_base, | ||
| 'aws': ['boto3>=1.9,<2'], | ||
| 'azure': [ | ||
| 'azure-storage-blob>=12.3.2,<13', | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am vary of using uv in our tests, unless it were so common that our users were doing the same, and docs instructed users to do so.