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