tflobbe commented on code in PR #1773: URL: https://github.com/apache/solr/pull/1773#discussion_r1260043257
########## solr/modules/s3-repository/src/java/org/apache/solr/s3/S3OutputStream.java: ########## @@ -200,6 +202,10 @@ public MultipartUpload(String uploadId) { } void uploadPart(ByteArrayInputStream inputStream, long partSize) { + if (aborted) { Review Comment: It's true that this isn't consistent with the logic added in `close`. I kind of like throwing the error instead of just ignoring things because it otherwise hides the fact that the component is being misused (an upload after an abort). The reality is that none of the error paths should happen with the current code. I'm more inclined to throw the ISE exception in the `compoete()` if `aborted` than going the other way around. >It'd make the logic a bit simpler in close() as well. Maybe it's simpler if I check for `aborted` in `close()` too and handled separately? let me push a change and see -- 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: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org