Remove deprecated dead methods from PinotHelixResourceManager#18778
Remove deprecated dead methods from PinotHelixResourceManager#18778Akanksha-kedia wants to merge 2 commits into
Conversation
Three @deprecated methods have no callers outside the class and can be safely removed: - deleteSchema(Schema): replaced by deleteSchema(String). No external callers; the only non-test caller uses the String overload. - getSchemaForTableConfig(TableConfig): thin wrapper around ZKMetadataProvider.getTableSchema(). No external callers. - getServersForSegment(String, String): renamed to getServers(). No external callers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #18778 +/- ##
============================================
+ Coverage 64.78% 64.81% +0.03%
Complexity 1309 1309
============================================
Files 3380 3380
Lines 209540 209535 -5
Branches 32797 32797
============================================
+ Hits 135751 135815 +64
+ Misses 62863 62801 -62
+ Partials 10926 10919 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
xiangfu0
left a comment
xiangfu0
left a comment
There was a problem hiding this comment.
Found one high-signal compatibility issue; see inline comment.
| * @param schema The schema to be deleted. | ||
| * @return True on success, false otherwise. | ||
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
These deprecated overloads are still public methods on a released controller artifact, so removing them is a binary-incompatible change for embedders compiled against current Pinot jars. Callers that still invoke deleteSchema(Schema), getSchemaForTableConfig(TableConfig), or getServersForSegment(...) will start failing with NoSuchMethodError on upgrade. Pinot usually keeps the old entry points as delegating shims until there is an explicit compatibility-removal plan.
There was a problem hiding this comment.
Agreed — thank you for the catch. I'll restore deleteSchema(Schema), getSchemaForTableConfig(TableConfig), and getServersForSegment(...) as delegating stubs so binary compatibility is preserved for callers on released jars. Will update this PR.
|
@xiangfu0 @Jackie-Jiang all CI checks pass, review comments addressed. Please review when you get a chance. |
…bility Per xiangfu0's review: deleteSchema(Schema), getSchemaForTableConfig(TableConfig), and getServersForSegment(...) are public methods on a released controller artifact; removing them would cause NoSuchMethodError for callers compiled against older jars. Restore as delegating stubs pending an explicit removal plan.
|
Closing this PR per xiangfu0's review. The three deprecated methods ( |
3 participants
Description
Removes three
@Deprecatedmethods fromPinotHelixResourceManagerthat have no callers outside the class. This continues the pattern of cleaning up dead deprecated code (similar to #18699, #18472, #17072).Methods removed
deleteSchema(Schema)deleteSchema(String)StringoverloadgetSchemaForTableConfig(TableConfig)ZKMetadataProvider.getTableSchema()directlygetServersForSegment(String, String)getServers(String, String)All three methods were marked
@Deprecatedwith their successors already in use. Removing them reduces the API surface and eliminates dead code.Tests
No test changes needed — no test callers exist for these methods.
Checklist