LuciferYang commented on PR #12414: URL: https://github.com/apache/gravitino/pull/12414#issuecomment-5351831733
Done in b2a7cd138, with follow-up validation and tests in 65e24faf9. All three things you named came out as you described. `GravitinoSparkPlugin` now lives in each version module and builds a `SparkBindings` for the shared `GravitinoDriverPlugin`. FQN unchanged, so `spark.plugins` is the same for users, and the dependency runs version module to shared code with no import that resolves only after source-set composition. The authorization extension became a binding, which let both flavor copies move into their own version module. `spark40` is now empty and gone; `spark35` is down to the Paimon package, its only remaining user. A missing binding fails in the builder, and so does a duplicate or a blank one. `SparkBindings.build()` requires every kind except Paimon, which some builds legitimately omit. The blank check matters most for the authorization extension: Spark's own parser filters blank entries out of `spark.sql.extensions`, so a blank binding there would drop the extension and every session would run unauthorized, with no error and no log line. Both `Class` overloads are bounded now, `Class<? extends TableCatalog>` and `Class<? extends Function1<SparkSessionExtensions, Void>>`, so the compile-time check is real rather than just "the class exists". The dispatch is testable, and tested. `TestGravitinoDriverPlugin` builds bindings out of `org.example.*` names, so it asserts what the plugin does with what it is given rather than restating what a module ships: which class each provider resolves to, what `registerCatalog` writes under `spark.sql.catalog.<name>`, that a kind with no binding registers nothing, and that the Iceberg and Paimon opt-in flags gate both catalogs and session extensions. Each version module also has a test that builds its own plugin, so a forgotten catalog fails there rather than at `SparkSession` startup. Paimon stays a string in `v3.5`'s plugin, resolved by presence, for the reason it already was — and `v3.5` has a test that pins that string against the class it ships, since the compiler cannot. -- 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]
