Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions python/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ dependencies = [
"anthropic>=0.64.0",
"chromadb==1.0.21",
"onnxruntime<1.24.1;python_version<'3.11'",
"onnxruntime>=1.24.1;python_version>='3.11'",

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.

Actually, I didn't add onnxruntime>=1.24.1 for python 3.11 initially, but I found for python 3.11 in macOS & arm, uv will also use onnxruntime==1.23.2, even if onnxruntime==1.24.1 is available. So I finally add a minimum version requirement for onnxruntime in python 3.11.

But the minimum version requirement indeed break the build in macos & x86_64. Do you have any good solutions?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

uv will also use onnxruntime==1.23.2, even if onnxruntime==1.24.1 is available.

@wenjin272 Did you notice this in your local env or CI? Have you tried to remove .env folder and uv lock to start with a fresh build?

I currently don't have ARM version Python installed on my (Company)laptop, I can check if installing ARM version Python is accpetable in my side.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It looks like all CI passed without this onnxruntime issue, seems we are good?

The current failed CI test seems to be not related to this change, it failed in maven test phrase: Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.5.2:test (default-test) on project flink-agents-end-to-end-tests-resource-cross-language:

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.

uv will also use onnxruntime==1.23.2, even if onnxruntime==1.24.1 is available.

@wenjin272 Did you notice this in your local env or CI? Have you tried to remove .env folder and uv lock to start with a fresh build?

I currently don't have ARM version Python installed on my (Company)laptop, I can check if installing ARM version Python is accpetable in my side.

I noticed this both in my local env and CI, even if I remove the uv lock or try uv sync --upgrade.

For example, in ut test for python 3.11 on macos for this pr: https://github.com/apache/flink-agents/actions/runs/21851853229/job/63068025146?pr=525, the installed onnxruntime is 1.23.2
image

It actually won't cause any failure, but may be unexpected.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It actually won't cause any failure, but may be unexpected.

@wenjin272 You are right on onnxruntime==1.23.2 is automatically resolved and picked up by uv. I think this is because of uv's universal resolver no longer needs to fork based on current constraints:

  • Python <3.11: onnxruntime<1.24.1 (from pyproject.toml)
  • Python >=3.11: onnxruntime with no version pin (from chromadb)

Since 1.23.2 satisfies both, uv unifies on a single version rather than forking. I would say this is expected from uv perspective, and the version 1.23.2 should work well with all Python versions we supported today.

Note: that the root cause here is upstream: chromadb declares https://github.com/chroma-core/chroma/blob/main/pyproject.toml#L10 with no upper bound and no platform markers. This means chromadb is unaware that newer onnxruntime releases may drop support for certain platforms (like macOS x86_64 in 1.24.1). Ideally, chromadb should handle this with appropriate version bounds or environment markers so that downstream projects don't need to independently work around these compatibility gaps.


I don't see a hard blocker that we had to to remove the support on macOS x86_64. That said, I think we should continue to support macOS x86_64. There are still users(like us internally) replies on the builds from macOS x86_64.

@wenjin272 @xintongsong WDYT?

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.

Thanks for the nice discussion. Here are my two cents.

  1. We should definitely support both x86 and arm on macos
  2. IIUC, the two objective constraints that we need to strictly follow:
  • onnxruntime 1.24.1 does not support python 3.10
  • onnxruntime 1.24.1 does not support x86 on macos
  1. There's no strong demand for using onnxruntime 1.24.1+ on python 3.11

Based on these, I think the changes @alnzng proposed in this PR make sense. WDYT, @wenjin272?

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.

Thanks for the nice discussion. Here are my two cents.

  1. We should definitely support both x86 and arm on macos
  2. IIUC, the two objective constraints that we need to strictly follow:
  • onnxruntime 1.24.1 does not support python 3.10
  • onnxruntime 1.24.1 does not support x86 on macos
  1. There's no strong demand for using onnxruntime 1.24.1+ on python 3.11

Based on these, I think the changes @alnzng proposed in this PR make sense. WDYT, @wenjin272?

make sense

]

[tool.setuptools]
Expand Down Expand Up @@ -110,7 +109,7 @@ members = ["."]
[tool.uv]
# Configure required environments for platform compatibility
required-environments = [
"sys_platform == 'darwin' and platform_machine == 'arm64'",
"sys_platform == 'darwin'",
"sys_platform == 'linux'",
"sys_platform == 'win32'",
]
Expand Down
1 change: 1 addition & 0 deletions tools/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ if $build_python; then
cd python
rm -rf dist/ # Clean old build artifacts before building
pip install uv
uv lock
uv sync --extra dev
uv run python -m build
uv pip install dist/*.whl
Expand Down