Copilot commented on code in PR #12708:
URL: https://github.com/apache/gravitino/pull/12708#discussion_r3878908360
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnectorFactory.java:
##########
@@ -51,7 +51,7 @@
public class GravitinoConnectorFactory implements ConnectorFactory {
private static final Logger LOG =
Logger.get(GravitinoConnectorFactory.class);
- private static final int MIN_SUPPORT_TRINO_SPI_VERSION = 435;
+ private static final int MIN_SUPPORT_TRINO_SPI_VERSION = 440;
private static final int MAX_SUPPORT_TRINO_SPI_VERSION = Integer.MAX_VALUE;
Review Comment:
The minimum supported Trino version was raised to 440 here, but the build
default in `gradle.properties` still sets `minSupportedTrinoVersion` to 435.
That means the default build of `:trino-connector:trino-connector` will still
compile against Trino 435 unless overridden, which is inconsistent with the new
minimum support policy and can mask incompatibilities.
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnectorFactory.java:
##########
@@ -51,7 +51,7 @@
public class GravitinoConnectorFactory implements ConnectorFactory {
private static final Logger LOG =
Logger.get(GravitinoConnectorFactory.class);
- private static final int MIN_SUPPORT_TRINO_SPI_VERSION = 435;
+ private static final int MIN_SUPPORT_TRINO_SPI_VERSION = 440;
private static final int MAX_SUPPORT_TRINO_SPI_VERSION = Integer.MAX_VALUE;
Review Comment:
The minimum supported Trino version changed (435 -> 440), but there is no
unit test asserting that version validation rejects 439 and accepts 440. Please
add a focused test that exercises `create(...)`/version validation with a
mocked `ConnectorContext.getSpiVersion()` at the boundary values so this policy
change is enforced by CI.
##########
trino-connector/integration-test/trino-test-tools/run_test_with_versions.sh:
##########
@@ -23,16 +23,16 @@
#
# Arguments:
# --trino_versions_map: Space-separated list of "version:module" pairs.
-# Defaults to "435:trino-connector" if not provided.
+# Defaults to "440:trino-connector" if not provided.
# --trino_test_args: Arguments passed directly to the underlying test
script.
#
# Example:
# ./run_test_with_versions.sh \
-# --trino_versions_map="435:trino-connector 478:trino-connector-470-478" \
+# --trino_versions_map="440:trino-connector 478:trino-connector-470-478" \
Review Comment:
This usage example references `trino-connector-470-478`, but that
module/directory does not exist in the build (the repo has
`trino-connector-473-478`). Copy/pasting this example will fail.
This issue also appears on line 36 of the same file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
