driessamyn-sparta commented on code in PR #27700:
URL: https://github.com/apache/flink/pull/27700#discussion_r3073897032
##########
flink-filesystems/flink-azure-fs-hadoop/pom.xml:
##########
@@ -150,6 +150,12 @@ under the License.
<pattern>org.apache.flink.runtime.util</pattern>
<shadedPattern>org.apache.flink.fs.azure.common</shadedPattern>
</relocation>
+ <!-- Relocate
Jackson to prevent conflicts with user jobs that depend on newer
Review Comment:
Thanks for the review @featzhang.
Relocating the full `com.fasterxml.jackson` namespace is consistent with how
Jackson is shaded across the rest of the Flink codebase — modules like
`flink-kubernetes`, `flink-sql-avro`, `flink-python`, and `flink-model-triton`
all relocate the same top-level package without a `ServicesResourceTransformer`.
Jackson's META-INF/services files (e.g. for `JsonFactory`, `ObjectCodec`)
are not relied upon at runtime in this context — the AWS SDK and Hadoop
instantiate Jackson classes directly rather than through the ServiceLoader
mechanism. In fact, `flink-kubernetes` explicitly excludes Jackson SPI files
from its shaded JAR (`<exclude>META-INF/services/*com.fasterxml*</exclude>`).
The shade plugin also automatically rewrites service file contents during
relocation, so even if they were loaded, the references would point to the
relocated classes.
##########
flink-filesystems/flink-azure-fs-hadoop/pom.xml:
##########
@@ -150,6 +150,12 @@ under the License.
<pattern>org.apache.flink.runtime.util</pattern>
<shadedPattern>org.apache.flink.fs.azure.common</shadedPattern>
</relocation>
+ <!-- Relocate
Jackson to prevent conflicts with user jobs that depend on newer
+ Jackson
versions. -->
+ <relocation>
Review Comment:
Good suggestion @featzhang. I've expanded the comment in all five filesystem
plugin POMs to explain why per-plugin shading is preferred over reusing a
Flink-wide namespace (see f9564af).
##########
flink-filesystems/flink-gs-fs-hadoop/pom.xml:
##########
@@ -224,6 +224,12 @@ under the License.
<pattern>org.apache.flink.runtime.util</pattern>
<shadedPattern>org.apache.flink.fs.gs.org.apache.flink.runtime.util</shadedPattern>
</relocation>
+ <!-- Relocate
Jackson to prevent conflicts with user jobs that depend on newer
+ Jackson
versions. -->
+ <relocation>
Review Comment:
Thanks @featzhang. The new `org.apache.flink.fs.gs.shaded.jackson` namespace
is distinct from the existing relocations in this module, which target
`org.apache.flink.fs.gs.org.apache.flink.runtime.fs.hdfs` and
`org.apache.flink.fs.gs.org.apache.flink.runtime.util`. There is no prefix
overlap between these.
As for Google Cloud libraries that may bundle their own Jackson:
`google-cloud-storage` and `gcs-connector` bring in standard (unshaded) Jackson
under `com.fasterxml.jackson`, which is exactly what this relocation rule
captures and rewrites. Google's own internally-shaded dependencies (e.g.
`com.google.api.client.json`) use separate package namespaces that are
unaffected by this relocation. I've verified in the built JAR that no
`com.fasterxml.jackson` classes remain unrelocated.
--
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]
