jonathansudhakar1 commented on issue #25769: URL: https://github.com/apache/beam/issues/25769#issuecomment-6690658335
@tvalentyn, I'd like to pick this up. Here's what I found on current master, and one question before I send a PR. **What the sinks return today.** `WriteToText`, `WriteToAvro`, `WriteToParquet`, `WriteToParquetBatched` and `WriteToTFRecord` all end in `iobase.Write(FileBasedSink)`. That returns the `PCollection[str]` emitted by the `FinalizeWrite` step: the final path of each shard, yielded after the rename ([filebasedsink.py#L434](https://github.com/apache/beam/blob/d8eb82acf2caac44e91e7d20c45070e12860b97e/sdks/python/apache_beam/io/filebasedsink.py#L434)). **Proposed docs change.** Add a short "Returns" note to those five docstrings, plus `iobase.Write`: the transform outputs a `PCollection` of the final file paths, one per shard, once they're in place. Add a pipeline test that `assert_that`s the output of `WriteToText(..., num_shards=3)` equals the three shard paths, so the documented contract is covered. **Question: retries.** When finalize runs again after a successful rename, already-renamed shards are skipped and not re-emitted ([filebasedsink.py#L331-L334](https://github.com/apache/beam/blob/d8eb82acf2caac44e91e7d20c45070e12860b97e/sdks/python/apache_beam/io/filebasedsink.py#L331-L334)). `test_file_sink_writing` asserts this: the retried `finalize_write` returns `[]` ([filebasedsink_test.py#L129-L137](https://github.com/apache/beam/blob/d8eb82acf2caac44e91e7d20c45070e12860b97e/sdks/python/apache_beam/io/filebasedsink_test.py#L129-L137)). If a runner retries the finalize bundle after the renames succeeded, the output can therefore miss files. Should I: 1. document this as a caveat (the output lists the files finalized by the successful attempt, which on retry may not be all of them), or 2. also emit those already-finalized shards so the output is always complete, changing that test's expectation? I'd lean toward (1) for a docs PR and (2) as a separate change, if you think (2) is wanted. .take-issue -- 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]
