Skip to content

Drop unused/mis-scoped deps in sandbox plugins#21729

Draft
bowenlan-amzn wants to merge 1 commit into
opensearch-project:mainfrom
bowenlan-amzn:audit/sandbox-plugin-deps
Draft

Drop unused/mis-scoped deps in sandbox plugins#21729
bowenlan-amzn wants to merge 1 commit into
opensearch-project:mainfrom
bowenlan-amzn:audit/sandbox-plugin-deps

Conversation

@bowenlan-amzn
Copy link
Copy Markdown
Member

Summary

Dependency audit of sandbox plugins — remove unused declarations and fix mis-scoped ones.

  • analytics-backend-datafusion: Drop redundant log4j-core compileOnly (PluginBuildPlugin already provides it). Rescope arrow-memory-unsafe from compileOnly to testRuntimeOnly (needed by ServiceLoader under flat test classpath, not used in main code). Add WHY comment on jackson-datatype-jdk8 (substrait-core <clinit> loads Jdk8Module).
  • analytics-backend-lucene: Drop testRuntimeOnly guava + failureaccess (already on testRuntimeClasspath via testImplementation analytics-engineapi analytics-frameworkruntimeOnly guava). Drop testCompileOnly immutables value-annotations (zero test usage, stale comment).

Test plan

  • ./gradlew :sandbox:plugins:analytics-backend-datafusion:precommit — PASS
  • ./gradlew :sandbox:plugins:analytics-backend-datafusion:test — PASS (all 200+ tests)
  • ./gradlew :sandbox:plugins:analytics-backend-lucene:precommit — PASS
  • ./gradlew :sandbox:plugins:analytics-backend-lucene:test — PASS
  • dependencyInsight confirms arrow-memory-unsafe and guava reach testRuntimeClasspath via correct paths
  • Plugin zip contents unchanged (no accidental bundling)

Per /Users/bowenlan/notes/Projects/mustang/build/audit-results:

analytics-backend-datafusion:
- Drop log4j-core compileOnly (PluginBuildPlugin provides it).
- Rescope arrow-memory-unsafe compileOnly → testRuntimeOnly: no main-code
  use; unit tests need an AllocationManager via ServiceLoader under the flat
  test classpath. Sibling plugins that declare compileOnly project(':plugins:arrow-base')
  directly get arrow-memory-netty via OpenSearch's resolveableCompileOnly
  wiring; datafusion goes through analytics-engine as middleman and misses
  that edge.

parquet-data-format:
- Drop arrow-memory-unsafe testRuntimeOnly: arrow-base's api ships
  arrow-memory-netty which satisfies ServiceLoader under test.

analytics-backend-lucene:
- Drop testRuntimeOnly guava + failureaccess: analytics-framework's
  runtimeOnly guava leaks via api project dep onto this plugin's
  runtimeClasspath, which testRuntimeClasspath extends.
- Drop testCompileOnly immutables value-annotations: no test usage,
  stale comment.

Verified: precommit + test (all 3) + bundlePlugin for all 3. Plugin zips
byte-identical in content.

Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
@github-actions
Copy link
Copy Markdown
Contributor

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 47441a0.

PathLineSeverityDescription
sandbox/plugins/analytics-backend-datafusion/build.gradle53highDependency removed: 'org.apache.logging.log4j:log4j-core' dropped from compileOnly scope. Maintainers should verify this removal does not silently pull in a different transitive version at runtime.
sandbox/plugins/analytics-backend-datafusion/build.gradle60highDependency scope changed: 'org.apache.arrow:arrow-memory-unsafe' moved from compileOnly to testRuntimeOnly. Any dependency scope change must be verified to ensure it does not introduce an unintended artifact resolution path.
sandbox/plugins/analytics-backend-lucene/build.gradle38highDependency removed: 'com.google.guava:guava' dropped from testRuntimeOnly scope. Maintainers should confirm no test relies on this being present at runtime and that removal does not mask a transitive dependency gap.
sandbox/plugins/analytics-backend-lucene/build.gradle39highDependency removed: 'com.google.guava:failureaccess:1.0.2' dropped from testRuntimeOnly scope. Pinned-version removals should be verified; this artifact is a required companion to Guava and its absence may cause NoClassDefFoundError at test runtime.
sandbox/plugins/analytics-backend-lucene/build.gradle42highDependency removed: 'org.immutables:value-annotations:2.8.8' dropped from testCompileOnly scope. Maintainers should verify no generated or annotation-processed code still depends on these annotations during test compilation.

The table above displays the top 10 most important findings.

Total: 5 | Critical: 0 | High: 5 | Medium: 0 | Low: 0


Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.


⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant