Move all analyticscore references behind a AnalyticsCoreUtil class

[talatuyarer]

We dont want to load analytics classes if users does not use analytic feature.

https://lists.apache.org/thread/mv0xhgoqvj2doh74g4wr8pgft3ozj0hz

[kevinjqliu]

i grepped for import.*analyticscore, looks like theres still an import in gcp/src/main/java/org/apache/iceberg/gcp/gcs/GcsInputStreamWrapper.java
i would expect all the imports for analytics-core isolated in AnalyticsCoreUtil.java

EDIT: ok looks like GcsInputStreamWrapper is only called by AnalyticsCoreUtil. no issues here

[RussellSpitzer]

i grepped for import.*analyticscore, looks like theres still an import in gcp/src/main/java/org/apache/iceberg/gcp/gcs/GcsInputStreamWrapper.java i would expect all the imports for analytics-core isolated in AnalyticsCoreUtil.java

EDIT: ok looks like GcsInputStreamWrapper is only called by AnalyticsCoreUtil. no issues here

This is actually a good call, you might as well make GcsinputStreamwrapper an inner class of AnalyticsCoreUtil and remove the possibility of another caller accidentally using it.

[kevinjqliu]

i tested this locally by adding this to the build.gradle's `iceberg-gcp' entry:

  task testWithoutAnalyticsCore(type: Test) {
    useJUnitPlatform()
    description = 'Runs tests without gcs-analytics-core on the classpath'
    classpath = test.classpath.filter {
      // exclude all JARs from the com.google.cloud.gcs.analytics group (analyticscore classes)
      !it.path.contains('com.google.cloud.gcs.analytics/')
    }
    jvmArgs += project.property('extraJvmArgs')
    include '**/TestGCSFileIO.class'
    include '**/TestGCSInputStream.class'
    include '**/TestGCSOutputStream.class'
    include '**/TestGCSLocation.class'
  }

This filters out the analytics-core library. And everything ran successfully.

[talatuyarer]

I believe I have addressed all comments.

[kevinjqliu]

LGTM

have a few minor nits

[aihuaxu]

Here better to return GcsFileSystem rather than AutoCloseable.

[talatuyarer]

Returning GcsFileSystem would expose the analytics-core class in the method descriptor, which is the opposite of what we're trying to achieve (e.g., PrefixedStorage.gcsFileSystem()). Could you clarify if there's another reason for the suggestion? @aihuaxu

[aihuaxu]

@talatuyarer I think you can return GcsFileSystem here but the caller can still use AutoCloseable, right?

But it's nit.

[aihuaxu]

LGTM.

[kevinjqliu]

LGTM thanks for fixing this

[kevinjqliu]

Thanks for the fix @talatuyarer and everyone for reviewing