kevinjqliu commented on code in PR #2609:
URL: https://github.com/apache/iceberg-python/pull/2609#discussion_r2430700862
##########
pyiceberg/table/__init__.py:
##########
@@ -1623,9 +1623,9 @@ def _metadata_location_from_version_hint(cls,
metadata_location: str, properties
if content.endswith(".metadata.json"):
return os.path.join(metadata_location, "metadata", content)
elif content.isnumeric():
- return os.path.join(metadata_location, "metadata",
"v%s.metadata.json").format(content)
+ return os.path.join(metadata_location, "metadata",
f"v{content}.metadata.json")
else:
- return os.path.join(metadata_location, "metadata",
"%s.metadata.json").format(content)
+ return os.path.join(metadata_location, "metadata",
f"{content}.metadata.json")
Review Comment:
nit: i think the version hint file's content should only be numeric value,
following the [HadoopTableOperations
implementation](https://github.com/apache/iceberg/blob/8342cbedd576142f55cbdfac33c4d339471b7832/core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java#L306-L320)
we can address this issue separately.
alternatively, since this is read-side only, we _could_ allow reading
non-numeric values.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
