Refactor: use upstream udf-runner-cpp base for Python VM - #6
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e7762a73c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| RUN mkdir /exasol_emulator | ||
| COPY emulator/ /exasol_emulator | ||
| COPY /exaudfclient/base/exaudflib/zmqcontainer.proto /exasol_emulator | ||
| COPY /exaudfclient/base/zmqcontainer.proto /exasol_emulator |
There was a problem hiding this comment.
The newly selected fixture ends inside exascript_response without a closing brace (and also drops the ping and call response fields used by emulator/exasolution.py). Consequently, the immediately following protoc command exits with Reached end of input in message definition (missing '}'), so every base_test_build_run image updated to copy this file fails to build.
Useful? React with 👍 / 👎.
| bazel build --lockfile_mode=off --config no-tty -c dbg --config python --config fast-binary --verbose_failures | ||
| working-directory: ./exaudfclient/ | ||
|
|
||
| tests: |
There was a problem hiding this comment.
we probably can remove this for the moment, because we don't have python cpp tests
There was a problem hiding this comment.
this comment was resolved but not fixed. Currently this job executes the bazel test from udf-runner-cpp and we don't have bazel tests for udf-runner-python, so we can remove this job.
| "replace_swig_import_helper.py"]) | ||
|
|
||
| load("//:variables.bzl", "VM_ENABLED_DEFINES") | ||
| +load("@rules_cc//cc:defs.bzl", "cc_library") |
There was a problem hiding this comment.
this we should fix in udf-runner-cpp
Co-authored-by: Torsten Kilias <tkilias@users.noreply.github.com>
3 participants
Summary
This PR replaces the locally vendored UDF client transport/base with the upstream
exasol/udf-runner-cppv1 base Bazel module. It retains only the Python VM and the minimal local adapter needed to instantiate it.What changed
exaudfclient_basethrough Bzlmod to upstream commitb3a8c204a2555b373c66110d5e8301a374b32a6a, usingudf-runner-cpp/v1/baseas the module root./exaudf/exaudfclientandlang=python.zmqcontainer.protodirectly from the pinned upstream commit, then generate its Python protobuf binding withprotoc; the local schema fixture is removed.Implementation plan completed
git_override, instead of the localbasemodule.lang=pythonand remove local streaming/benchmark VM support.Validation
git diff --check.bazel mod graph.bazel mod show_repo exaudfclient_base; it resolves the requested remote, commit, andudf-runner-cpp/v1/basestrip prefix.Known limitation
A full local compile was not run: this workstation has Bazel 9.2, while the pinned upstream v1 base targets Bazel 7.2.1. Bazel 9 rejects an upstream legacy built-in rule before analyzing the local Python target. The project container/CI toolchain uses the supported Bazel 7 line.
Original prompt