lidavidm commented on code in PR #824:
URL: https://github.com/apache/arrow-go/pull/824#discussion_r3295390007
##########
parquet/file/file_writer.go:
##########
@@ -65,11 +65,39 @@ func WithWriteMetadata(meta metadata.KeyValueMetadata)
WriteOption {
}
}
-// NewParquetWriter returns a Writer that writes to the provided WriteSeeker
with the given schema.
+// NewParquetWriter returns a Writer that writes to the provided io.Writer
with the given schema.
//
// If props is nil, then the default Writer Properties will be used. If the
key value metadata is not nil,
// it will be added to the file.
+//
+// This constructor panics with "failed to write magic number" if the
+// initial 4-byte write of the parquet magic header to the underlying sink
+// fails. Callers using a sink that may transiently fail (for example, a
+// cloud-storage upload writer or other network-attached writer) should
+// prefer [NewParquetWriterWithError], which surfaces the failure as a
+// returned error instead.
func NewParquetWriter(w io.Writer, sc *schema.GroupNode, opts ...WriteOption)
*Writer {
Review Comment:
Should we deprecate this as well?
##########
parquet/file/file_writer.go:
##########
Review Comment:
For someone coming to the codebase _without_ being a maintainer, these doc
comments are too focused on historical knowledge. I think
NewParquetWriterWithError should be documented without reference to
NewParquetWriter and simply mention that it may fail if it cannot write the
header (e.g. because of a flaky writer), and NewParquetWriter can document the
history if needed.
--
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]
