amogh-jahagirdar commented on code in PR #11982:
URL: https://github.com/apache/iceberg/pull/11982#discussion_r1926310867
##########
spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/RewriteTablePathSparkAction.java:
##########
@@ -728,4 +710,13 @@ private String getMetadataLocation(Table tbl) {
!metadataDir.isEmpty(), "Failed to get the metadata file root
directory");
return metadataDir;
}
+
+ @VisibleForTesting
+ Broadcast<Table> tableBroadcast() {
+ if (tblBroadcast == null) {
Review Comment:
Generally in the project we try and use the full name where possible,
`tableBroadcast`.
Also I'm not really against it, but now that we know that we're just using
specs from serializable table , I'm not really sure how valuable it is to
expose this visible for testing method and have the test for just getting the
spec value from broadcast. I think it was useful to surface the issue but just
not sure what we get out of having it constantly run in tests. But considering
it's a lightweight test I'm not against it, just seems a bit out of place.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
