xintongsong commented on code in PR #23072:
URL: https://github.com/apache/flink/pull/23072#discussion_r1307140125
##########
flink-core/src/main/java/org/apache/flink/core/fs/Path.java:
##########
@@ -443,6 +443,7 @@ public Path makeQualified(FileSystem fs) {
// Legacy Serialization
// ------------------------------------------------------------------------
+ @Deprecated
Review Comment:
We'd better add a java doc to explain that:
1. Path will no longer implement `IOReadableWritable` in future versions.
(We probably should also add this for the class JavaDoc.)
2. Caller should use `deserializeFromDataInputView` instead.
3. Per FLIP-321, we should add a comment "Since X.X.X" to indicate when was
the API deprecated.
##########
flink-core/src/main/java/org/apache/flink/core/fs/Path.java:
##########
@@ -526,4 +528,55 @@ private boolean hasWindowsDrive(String path, boolean
slashed) {
public static Path fromLocalFile(File file) {
return new Path(file.toURI());
}
+
+ /**
+ * Deserialize the Path from {@link DataInputView}.
+ *
+ * @param in the data input view.
+ * @return the path
+ * @throws IOException if an error happened.
+ */
+ public static Path deserializeFromDataInputView(DataInputView in) throws
IOException {
Review Comment:
Should deduplicate, by calling `deserializeFromDataInputView` from `read`.
##########
flink-core/src/main/java/org/apache/flink/core/fs/Path.java:
##########
@@ -526,4 +528,55 @@ private boolean hasWindowsDrive(String path, boolean
slashed) {
public static Path fromLocalFile(File file) {
return new Path(file.toURI());
}
+
+ /**
+ * Deserialize the Path from {@link DataInputView}.
+ *
+ * @param in the data input view.
+ * @return the path
+ * @throws IOException if an error happened.
+ */
+ public static Path deserializeFromDataInputView(DataInputView in) throws
IOException {
+ final boolean isNotNull = in.readBoolean();
+ Path result = new Path();
+ if (isNotNull) {
+ final String scheme = StringUtils.readNullableString(in);
+ final String userInfo = StringUtils.readNullableString(in);
+ final String host = StringUtils.readNullableString(in);
+ final int port = in.readInt();
+ final String path = StringUtils.readNullableString(in);
+ final String query = StringUtils.readNullableString(in);
+ final String fragment = StringUtils.readNullableString(in);
+
+ try {
+ result = new Path(new URI(scheme, userInfo, host, port, path,
query, fragment));
+ } catch (URISyntaxException e) {
+ throw new IOException("Error reconstructing URI", e);
+ }
+ }
+ return result;
Review Comment:
Shall we return `null` if `isNotNull` is false?
##########
flink-core/src/main/java/org/apache/flink/core/fs/Path.java:
##########
@@ -526,4 +528,55 @@ private boolean hasWindowsDrive(String path, boolean
slashed) {
public static Path fromLocalFile(File file) {
return new Path(file.toURI());
}
+
+ /**
+ * Deserialize the Path from {@link DataInputView}.
+ *
+ * @param in the data input view.
+ * @return the path
+ * @throws IOException if an error happened.
+ */
+ public static Path deserializeFromDataInputView(DataInputView in) throws
IOException {
+ final boolean isNotNull = in.readBoolean();
+ Path result = new Path();
+ if (isNotNull) {
+ final String scheme = StringUtils.readNullableString(in);
+ final String userInfo = StringUtils.readNullableString(in);
+ final String host = StringUtils.readNullableString(in);
+ final int port = in.readInt();
+ final String path = StringUtils.readNullableString(in);
+ final String query = StringUtils.readNullableString(in);
+ final String fragment = StringUtils.readNullableString(in);
+
+ try {
+ result = new Path(new URI(scheme, userInfo, host, port, path,
query, fragment));
+ } catch (URISyntaxException e) {
+ throw new IOException("Error reconstructing URI", e);
+ }
+ }
+ return result;
Review Comment:
Actually, I wonder if we should forbid null 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]
