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

Reply via email to