aicam commented on code in PR #4146:
URL: https://github.com/apache/texera/pull/4146#discussion_r2669178908
##########
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##########
Review Comment:
In `initMultipartUpload`, `validateAndNormalizeFilePathOrThrow` is used to
make sure physical address is correct, why we check here again? this is a
duplicate logic
##########
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##########
@@ -1399,7 +1466,8 @@ class DatasetResource {
private def initMultipartUpload(
did: Integer,
encodedFilePath: String,
- numParts: Optional[Integer],
+ fileSizeBytes: Optional[java.lang.Long],
Review Comment:
Why here the parameter receive `java.lang.Long` and then in line 1487 its
converted to Scala? I think its better to receive Scala long in the first place
##########
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##########
Review Comment:
Replace the `55P03` with a constant or something more meaningful
##########
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##########
@@ -715,7 +727,55 @@ class DatasetResource {
if (session == null)
throw new NotFoundException("Upload session not found. Call type=init
first.")
- val expectedParts = session.getNumPartsRequested
+ val expectedParts: Int = session.getNumPartsRequested
+ val fileSizeBytesValue: Long = session.getFileSizeBytes
+ val partSizeBytesValue: Long = session.getPartSizeBytes
+
+ if (fileSizeBytesValue <= 0L) {
Review Comment:
After failing to upload, the record should be deleted from database, I
suggest move error catching logics to a function, and if any of them failed,
just remove database records, this way you write the recycling logic once
--
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]
