anoopj commented on code in PR #16572:
URL: https://github.com/apache/iceberg/pull/16572#discussion_r3314374899
##########
core/src/test/java/org/apache/iceberg/rest/responses/TestLoadTableResponseParser.java:
##########
@@ -164,7 +164,7 @@ public void roundTripSerdeV3andHigher(int formatVersion) {
+ " \"metadata\" : {\n"
+ " \"format-version\" : %s,\n"
+ " \"table-uuid\" :
\"386b9f01-002b-4d8c-b77f-42c3fd3b7c9b\",\n"
- + " \"location\" : \"location\",\n"
+ + " \"location\" : \"file:/location\",\n"
Review Comment:
Minor: `file:/location` is a valid location per RFC 3986, but it is a bit
unusual. Consdier using `file://location`?
##########
core/src/test/java/org/apache/iceberg/TestTables.java:
##########
@@ -133,7 +133,8 @@ private static TestTable createTable(
}
TableMetadata metadata =
- newTableMetadata(schema, spec, sortOrder, temp.toString(), properties,
formatVersion);
+ newTableMetadata(
+ schema, spec, sortOrder, temp.toURI().toString(), properties,
formatVersion);
Review Comment:
I think the change to use `toURI` in this class is necessitating
`URI.create(...)` in downstream tests. This is because the Java URI class
produces strings like `file:/location`, which `File` doesn't recognize. One
way to avoid this is to build the scheme-prefixed path manually (e.g.
`"file://" + temp.getAbsolutePath()`) in this class.
--
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]
