Upgrade protobuf generation code and add a new `generate_protos.py` script by WeichenXu123 · Pull Request #12360 · mlflow/mlflow

WeichenXu123 · 2024-06-14T09:55:35Z

🛠 DevTools 🛠

Install mlflow from this PR

pip install git+https://github.com/mlflow/mlflow.git@refs/pull/12360/merge

Checkout with GitHub CLI

gh pr checkout 12360

Related Issues/PRs

closes #11811

What changes are proposed in this pull request?

Since protobuf python runtime version 5.26.0, it is not compatible with python gencode generated by protoc 3.19.4,
and according to https://protobuf.dev/support/cross-version-runtime-guarantee/#backwards,
New Gencode + Old Runtime is not allowed.
So to make MLflow supports either protobuf runtime version >= 5.26 or < 5.26, I make each python protobuf gencode module like:

import google.protobuf
from packaging.version import Version
if Version(google.protobuf.__version__) >= Version("5.26.0"):
   # code generated by protoc 5.26.0
else:
   # code generated by protoc 3.19.0

I also write a new generate_protos.py script so that it can automatically generates above multiple version bundled gencode python module.

How is this PR tested?

Existing unit/integration tests
New unit/integration tests
Manual tests

Does this PR require documentation update?

Release Notes

Is this a user-facing change?

No. You can skip the rest of this section.
Yes. Give a description of this change to be included in the release notes for MLflow users.

What component(s), interfaces, languages, and integrations does this PR affect?

Components

Interface

area/uiux: Front-end, user experience, plotting, JavaScript, JavaScript dev server
area/docker: Docker use across MLflow's components, such as MLflow Projects and MLflow Models
area/sqlalchemy: Use of SQLAlchemy in the Tracking Service or Model Registry
area/windows: Windows support

Language

language/r: R APIs and clients
language/java: Java APIs and clients
language/new: Proposals for new client languages

Integrations

integrations/azure: Azure and Azure ML integrations
integrations/sagemaker: SageMaker integrations
integrations/databricks: Databricks integrations

How should the PR be classified in the release notes? Choose one:

rn/none - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
rn/breaking-change - The PR will be mentioned in the "Breaking Changes" section
rn/feature - A new user-facing feature worth mentioning in the release notes
rn/bug-fix - A user-facing bug fix worth mentioning in the release notes
rn/documentation - A user-facing documentation change worth mentioning in the release notes

Should this PR be included in the next patch release?

Yes should be selected for bug fixes, documentation updates, and other small changes. No should be selected for new features and larger changes. If you're unsure about the release classification of this PR, leave this unchecked to let the maintainers decide.

What is a minor/patch release?

Minor release: a release that increments the second part of the version number (e.g., 1.2.0 -> 1.3.0).
Bug fixes, doc updates and new features usually go into minor releases.
Patch release: a release that increments the third part of the version number (e.g., 1.2.0 -> 1.2.1).
Bug fixes and doc updates usually go into patch releases.

Yes (this PR will be cherry-picked and included in the next patch release)
No (this PR will be included in the next minor release)

Signed-off-by: Weichen Xu <[email protected]>

WeichenXu123 · 2024-06-14T09:56:52Z

+import google.protobuf
+from packaging.version import Version
+if Version(google.protobuf.__version__) >= Version("5.26.0"):


Each pb module checks protobuf runtime version first, then executes matched gencode.

All these gencode (including multiple version gencode) are generated by generate_protos.py script.

Signed-off-by: Weichen Xu <[email protected]>

harupy · 2024-06-17T08:23:21Z

how do we ensure both the 5.26.0 code and the 3.19.0 code work?

WeichenXu123 · 2024-06-17T09:21:39Z

Note java pom.xml already shades google protobuf:
See

mlflow/mlflow/java/client/pom.xml

Line 101 in db9143d

<include>com.google.protobuf:protobuf-java</include>

We can verify it by command:

cd mlflow/java
mvn clean package -Dmaven.test.skip=true -Dproject.build.sourceEncoding=UTF-8

jar -tf client/target/mlflow-client-2.13.3-SNAPSHOT.jar | grep "mlflow_project/google/protobuf"

outputs:

org/mlflow_project/google/protobuf/
org/mlflow_project/google/protobuf/AbstractMessage$Builder.class
org/mlflow_project/google/protobuf/AbstractMessage$BuilderParent.class
org/mlflow_project/google/protobuf/AbstractMessage.class
org/mlflow_project/google/protobuf/AbstractMessageLite$Builder$LimitedInputStream.class
org/mlflow_project/google/protobuf/AbstractMessageLite$Builder.class
org/mlflow_project/google/protobuf/AbstractMessageLite$InternalOneOfEnum.class
org/mlflow_project/google/protobuf/AbstractMessageLite.class
org/mlflow_project/google/protobuf/AbstractParser.class
...

WeichenXu123 · 2024-06-17T09:21:53Z

how do we ensure both the 5.26.0 code and the 3.19.0 code work?

I will add CI for it.

BenWilson2 · 2024-06-18T00:56:53Z

+    ], "The script only supports MacOS or Linux system."
+    assert platform.machine() in [
+        "x86_64",
+        "aarch64",


Do we need to support arm64 CPU as well?

I don't have machine to test that. We can add it when we receive the use-case.

BenWilson2 · 2024-06-18T01:03:13Z

+    return "\n".join(lines)
+
+
+def generate_final_python_gencode(gencode3194_path, gencode5260_path, out_path):


This is clever

Signed-off-by: Weichen Xu <[email protected]>

github-actions · 2024-06-18T11:59:14Z

Documentation preview for 4ec2ec7 will be available when this CircleCI job
completes successfully.

More info

Ignore this comment if this PR does not change the documentation.
It takes a few minutes for the preview to be available.
The preview is updated when a new commit is pushed to this PR.
This comment was created by https://github.com/mlflow/mlflow/actions/runs/9606977090.

Signed-off-by: Weichen Xu <[email protected]>

harupy · 2024-06-20T16:24:58Z

/ai hello, fix this github action workflow. Please make the following updates:

protobuf_major_version_plus_one is confusing. Rename it to protobuf_major_version and increment the version before pip install.

@harupy

Sure, here's how you can update the code snippet:

jobs: core_tests: if: github.event_name != 'pull_request' || github.event.pull_request.draft == false runs-on: ubuntu-latest timeout-minutes: 120 strategy: fail-fast: false matrix: group: [1, 2] - protobuf_major_version_plus_one: [4, 5, 6] + protobuf_major_version: [3, 4, 5] include: - splits: 2 steps: - uses: actions/checkout@v4 with: submodules: recursive - uses: ./.github/actions/untracked - uses: ./.github/actions/free-disk-space - uses: ./.github/actions/setup-python - uses: ./.github/actions/setup-pyenv - uses: ./.github/actions/setup-java - name: Install dependencies run: | pip install --no-dependencies tests/resources/mlflow-test-plugin pip install .[extras] pip install pyspark # Install Hugging Face datasets to test Hugging Face usage with MLflow dataset tracking pip install datasets # Install TensorFlow to test TensorFlow dataset usage with MLflow dataset tracking pip install tensorflow # Install torch and transformers to test metrics pip install torch transformers pip install -r requirements/test-requirements.txt # Test against specific protobuf version - pip install "protobuf<${{ matrix.protobuf_major_version_plus_one }}" + protobuf_incremented_version=$((${{ matrix.protobuf_major_version }} + 1)) + pip install "protobuf<${protobuf_incremented_version}" - uses: ./.github/actions/pipdeptree - name: Run tests run: | pytest --splits=${{ matrix.splits }} --group=${{ matrix.group }} \ --ignore-flavors --ignore=tests/projects --ignore=tests/examples --ignore=tests/recipes --ignore=tests/evaluate --ignore tests/deployments/server \ tests

This changes the confusing protobuf_major_version_plus_one to protobuf_major_version and increments the version number before passing it to pip install.

Click here to see the usage data

{ "prompt_tokens": 893, "completion_tokens": 480, "total_tokens": 1373, "estimated_cost_in_usd": 0.05559 }

@WeichenXu123 you can try this. I think it works.

This change failed with error: https://github.com/mlflow/mlflow/actions/runs/9606163544

That's because you used ${{ protobuf_incremented_version }}, not ${protobuf_incremented_version} or $protobuf_incremented_version.

interesting , it failed again using ${protobuf_incremented_version}: https://github.com/mlflow/mlflow/actions/runs/9606814485/job/26496930964?pr=12360 @harupy

/home/runner/work/_temp/6ae75bcc-0fce-4971-9e2b-b916fb7e6c16.sh: line 13: protobuf<${ protobuf_incremented_version }: bad substitution

@WeichenXu123

% FOO=foo % echo "${ FOO }" zsh: bad substitution % echo "${FOO}" foo

this works too protobuf<$protobuf_incremented_version

Signed-off-by: Weichen Xu <[email protected]>

harupy · 2024-06-21T01:20:35Z

@BenWilson2 could you also take another look?

Signed-off-by: Weichen Xu <[email protected]>

harupy

LGTM

BenWilson2

LGTM once https://github.com/mlflow/mlflow/pull/12360/files#r1647840531 is addressed! Thanks for the clever implementation here @WeichenXu123 :D

harupy · 2024-06-21T01:43:52Z

+          pytest --splits=${{ matrix.splits }} --group=${{ matrix.group }} \
+            --ignore-flavors --ignore=tests/projects --ignore=tests/examples --ignore=tests/recipes --ignore=tests/evaluate --ignore tests/deployments/server \
+            tests


This can be a follow-up. does this run tests unrelated to protobuf?

yes. We can filter out tests unrelated to protobuf, but no simple rules currently

Signed-off-by: Weichen Xu <[email protected]>

WeichenXu123 · 2024-06-21T03:56:35Z

oh, I found an issue:

+ pip install 'protobuf<6'
Requirement already satisfied: protobuf<6 in /opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages (4.25.3)

I will file a follow-up PR.

villainb-dg · 2024-07-11T14:01:13Z

Any idea when this PR will be released? The upgrade of protobuf requirements to 6+ is blocking us from upgrade grpcio past 1.63.0. Thanks

BenWilson2 · 2024-07-11T18:17:15Z

@villainb-dg late next week :)

villainb-dg · 2024-07-22T08:28:57Z

@BenWilson2 So it seems that the latest release 4.12.3 did not include your change (https://github.com/mlflow/mlflow/blob/v2.14.3/pyproject.toml#L45). Any idea why?

WeichenXu123 added 2 commits June 14, 2024 17:40

init

32dc9d7

Signed-off-by: Weichen Xu <[email protected]>

test_message_pb2 gen

211164e

Signed-off-by: Weichen Xu <[email protected]>

WeichenXu123 commented Jun 14, 2024

View reviewed changes

WeichenXu123 added 4 commits June 14, 2024 17:59

clean

c8b3cd0

Signed-off-by: Weichen Xu <[email protected]>

update script

26f85b5

Signed-off-by: Weichen Xu <[email protected]>

update yaml

8244dc7

Signed-off-by: Weichen Xu <[email protected]>

format

25d75d2

Signed-off-by: Weichen Xu <[email protected]>

WeichenXu123 requested a review from mlflow-automation June 14, 2024 13:16

mlflow-automation requested review from B-Step62, BenWilson2, daniellok-db, harupy and serena-ruan and removed request for mlflow-automation June 14, 2024 13:16

BenWilson2 reviewed Jun 18, 2024

View reviewed changes

WeichenXu123 added 2 commits June 18, 2024 19:44

Merge branch 'master' into upgrade-proto2

8db9537

add-ci

68cac57

Signed-off-by: Weichen Xu <[email protected]>

WeichenXu123 added 5 commits June 18, 2024 20:08

update

d5dcb34

Signed-off-by: Weichen Xu <[email protected]>

update

9c245ad

Signed-off-by: Weichen Xu <[email protected]>

update

77d5844

Signed-off-by: Weichen Xu <[email protected]>

update

817f3e0

Signed-off-by: Weichen Xu <[email protected]>

path-filter

6a63843

Signed-off-by: Weichen Xu <[email protected]>

WeichenXu123 requested a review from BenWilson2 June 19, 2024 09:50

github-actions Bot added area/tracking Tracking service, tracking client APIs, autologging and removed area/tracking Tracking service, tracking client APIs, autologging labels Jun 19, 2024

harupy reviewed Jun 20, 2024

View reviewed changes

Comment thread .github/workflows/protobuf-cross-test.yml Outdated

harupy reviewed Jun 20, 2024

View reviewed changes

Comment thread .github/workflows/protobuf-cross-test.yml Outdated

harupy reviewed Jun 20, 2024

View reviewed changes

Comment thread dev/generate_protos.py Outdated

harupy reviewed Jun 20, 2024

View reviewed changes

Comment thread dev/generate_protos.py

harupy reviewed Jun 20, 2024

View reviewed changes

Comment thread dev/generate_protos.py Outdated

WeichenXu123 added 6 commits June 20, 2024 22:16

update

ecd24d0

Signed-off-by: Weichen Xu <[email protected]>

update

136d883

Signed-off-by: Weichen Xu <[email protected]>

update-s

71d4bd4

update

c502b5b

Signed-off-by: Weichen Xu <[email protected]>

fix

2617d13

Signed-off-by: Weichen Xu <[email protected]>

fix ci

3909054

Signed-off-by: Weichen Xu <[email protected]>

harupy reviewed Jun 20, 2024

View reviewed changes

WeichenXu123 added 2 commits June 21, 2024 08:15

update

71a4665

Signed-off-by: Weichen Xu <[email protected]>

format

1a3b858

Signed-off-by: Weichen Xu <[email protected]>

harupy reviewed Jun 21, 2024

View reviewed changes

Comment thread dev/generate_protos.py Outdated

harupy reviewed Jun 21, 2024

View reviewed changes

Comment thread dev/generate_protos.py

harupy reviewed Jun 21, 2024

View reviewed changes

Comment thread .github/workflows/protobuf-cross-test.yml Outdated

update

c32d71d

Signed-off-by: Weichen Xu <[email protected]>

harupy approved these changes Jun 21, 2024

View reviewed changes

BenWilson2 approved these changes Jun 21, 2024

View reviewed changes

harupy reviewed Jun 21, 2024

View reviewed changes

update

4ec2ec7

Signed-off-by: Weichen Xu <[email protected]>

WeichenXu123 merged commit a4c7f21 into mlflow:master Jun 21, 2024

WeichenXu123 mentioned this pull request Jun 21, 2024

Fix protobuf cross test versions #12443

Merged

39 tasks

Joelius300 mentioned this pull request May 21, 2025

Dependency updates & compatibility errors (re-training needed?) SeldonIO/MLServer#2151

Open

		return "\n".join(lines)


		def generate_final_python_gencode(gencode3194_path, gencode5260_path, out_path):

Uh oh!

Conversation

WeichenXu123 commented Jun 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Install mlflow from this PR

Checkout with GitHub CLI

Related Issues/PRs

What changes are proposed in this pull request?

How is this PR tested?

Does this PR require documentation update?

Release Notes

Is this a user-facing change?

What component(s), interfaces, languages, and integrations does this PR affect?

How should the PR be classified in the release notes? Choose one:

Should this PR be included in the next patch release?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harupy commented Jun 17, 2024

Uh oh!

WeichenXu123 commented Jun 17, 2024

Uh oh!

WeichenXu123 commented Jun 17, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jun 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlflow Bot Jun 20, 2024

Choose a reason for hiding this comment

Uh oh!

harupy Jun 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WeichenXu123 Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harupy Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WeichenXu123 Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

harupy commented Jun 21, 2024

Uh oh!

harupy left a comment

Choose a reason for hiding this comment

Uh oh!

BenWilson2 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WeichenXu123 commented Jun 14, 2024 •

edited

Loading

github-actions Bot commented Jun 18, 2024 •

edited

Loading

harupy Jun 20, 2024 •

edited

Loading

WeichenXu123 Jun 21, 2024 •

edited

Loading

harupy Jun 21, 2024 •

edited

Loading

WeichenXu123 Jun 21, 2024 •

edited

Loading