kevinjqliu commented on code in PR #1642:
URL: https://github.com/apache/iceberg-python/pull/1642#discussion_r1955380186


##########
pyiceberg/table/locations.py:
##########
@@ -64,6 +71,35 @@ def new_data_location(self, data_file_name: str, 
partition_key: Optional[Partiti
             str: A fully-qualified location URI for the data file.
         """
 
+    def new_table_metadata_file_location(self, new_version: int = 0) -> str:

Review Comment:
   oh i see the semantic difference. 
   
   `new_data_location` is an abstract method, which allows custom 
LocationProviders to override. 
   `load_location_provider` returns `SimpleLocationProvider` by default, which 
then use the `SimpleLocationProvider`'s `new_data_location` method in most 
cases. 
   
   we can do the same for `new_metadata_location`, making it an abstract method 
so that custom LocationProviders can override. but also provide a default 
implementation for both `SimpleLocationProvider` and 
`ObjectStoreLocationProvider` 
   
   we can then mention in the LocationProvider documentation that 
`new_metadata_location` can be overriden 
   
   Does this make sense?  @smaheshwar-pltr is this what you were pointing out? 
   
   



-- 
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

Reply via email to