Skip to content

Conversation

@yjshen
Copy link
Contributor

@yjshen yjshen commented Mar 23, 2023

No description provided.

}

/// Write data into an open file.
pub fn write(&self, buf: &[u8]) -> Result<i32, HdfsErr> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To align with the semantics of LocalFileSystem write, and imitate the behavior of https://doc.rust-lang.org/src/std/io/mod.rs.html#1540-1555, which actually allows an empty buffer as input, but writes the whole buffer out.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As

fs-hdfs/c_src/libhdfs/hdfs.c

Lines 1464 to 1466 in cadaee5

// Unlike most Java streams, FSDataOutputStream never does partial writes.
// If we succeeded, all the data was written.
return length;
mentioned, the hdfsWrite won't write partial contents. Maybe it's better to refine the condition check as follows:
if written_len >= 0 { Ok(written_len) } else { Err(HdfsErr::Generic(format!( "Fail to write contents to file {}", self.path() ))) }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, will do

Copy link
Contributor Author

@yjshen yjshen Mar 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted, only allow to call with an empty buffer and do nothing for this case.


if ptr.is_null() {
Err(HdfsErr::Generic(format!(
Err(HdfsErr::FileNotFound(format!(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mistyped error.

fn new_hdfs_file(&self, path: &str, file: hdfsFile) -> Result<HdfsFile, HdfsErr> {
if file.is_null() {
Err(HdfsErr::Generic(format!(
Err(HdfsErr::FileNotFound(format!(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mistyped error.

Copy link
Collaborator

@yahoNanJing yahoNanJing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yahoNanJing yahoNanJing merged commit 0ad1398 into hadoop-3.1.4 Mar 27, 2023
yahoNanJing pushed a commit that referenced this pull request Mar 27, 2023
* Fix issues seen during delta-rs integration

* resolve coments: rm while

* minimal changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants