-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Check Connector version in SPI compatibility check #11951
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
Changes from all commits
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 |
|---|---|---|
|
|
@@ -13,8 +13,8 @@ | |
| */ | ||
| package io.trino.plugin.base; | ||
|
|
||
| import io.trino.spi.connector.Connector; | ||
| import io.trino.spi.connector.ConnectorContext; | ||
| import io.trino.spi.connector.ConnectorFactory; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
|
|
@@ -32,10 +32,10 @@ private Versions() {} | |
| * chooses not to maintain compatibility with older SPI versions, as happens for plugins maintained together with | ||
| * the Trino project. | ||
| */ | ||
| public static void checkSpiVersion(ConnectorContext context, ConnectorFactory connectorFactory) | ||
| public static void checkSpiVersion(ConnectorContext context, String connectorName, Class<? extends Connector> connectorClass) | ||
| { | ||
| String spiVersion = context.getSpiVersion(); | ||
| Optional<String> pluginVersion = getPluginMavenVersion(connectorFactory); | ||
| Optional<String> pluginVersion = getPluginMavenVersion(connectorClass); | ||
|
|
||
| if (pluginVersion.isEmpty()) { | ||
| // Assume we're in tests. In tests, plugin version is unknown and SPI version may be known, e.g. when running single module's tests from maven. | ||
|
|
@@ -47,14 +47,14 @@ public static void checkSpiVersion(ConnectorContext context, ConnectorFactory co | |
| format( | ||
| "Trino SPI version %s does not match %s connector version %s. The connector cannot be used with SPI version other than it was compiled for.", | ||
| spiVersion, | ||
| connectorFactory.getName(), | ||
| connectorName, | ||
| pluginVersion.get())); | ||
| } | ||
|
|
||
| private static Optional<String> getPluginMavenVersion(ConnectorFactory connectorFactory) | ||
| private static Optional<String> getPluginMavenVersion(Class<? extends Connector> connectorClass) | ||
|
Member
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. Why "maven" version? This is I think this is independent of Maven and the name should be changed to not reflect the particular build system (again, this is a
Member
Author
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. That's Trino code. All Trino plugins use airbase and this depends on airbase's populating jar's manifest. (BTW I agree the original change (#11774) could use a better source of information -- the SPI version a connector was compiled against -- instead of connector's version. However, i didn't find a way to do this without changing each of the connectors. From Trino project's perspective, it would be an unnecessary complexity anyway, since SPI version is the same as the connector's version)
Member
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. It's really easy to do (if you know how, and luckily there is prior art) Connectors which want an SPI version match should have a Maven Transformation in their pom which writes some "attribute" to the Manifest, and, if present, Trino enforces a match. And we write tests for the match. And we add a flag to disable the match (for CI, or testing upgrades without having to change versions, etc...). |
||
| { | ||
| String specificationVersion = connectorFactory.getClass().getPackage().getSpecificationVersion(); | ||
| String implementationVersion = connectorFactory.getClass().getPackage().getImplementationVersion(); | ||
| String specificationVersion = connectorClass.getPackage().getSpecificationVersion(); | ||
| String implementationVersion = connectorClass.getPackage().getImplementationVersion(); | ||
| if (specificationVersion == null && implementationVersion == null) { | ||
| // The information comes from jar's Manifest and is not present e.g. when running tests, or from an IDE. | ||
| return Optional.empty(); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.