Copilot commented on code in PR #5911:
URL: https://github.com/apache/texera/pull/5911#discussion_r3463014001
##########
common/workflow-core/src/main/scala/org/apache/texera/amber/core/storage/FileResolver.scala:
##########
@@ -75,9 +75,13 @@ object FileResolver {
filePath.toUri
}
+ private val RESOURCE_TYPE_PREFIXES = Set("datasets")
+
/**
* Parses a dataset file path and extracts its components.
- * Expected format: /ownerEmail/datasetName/versionName/fileRelativePath
+ * Expected format:
/datasets/ownerEmail/datasetName/versionName/fileRelativePath
+ *
+ * The first segment is a resource type prefix (e.g. "datasets") and is
stripped before parsing.
*
Review Comment:
The ScalaDoc says the expected format is always `/datasets/...`, but
`parseDatasetFilePath` still accepts the legacy unprefixed format (it only
strips the prefix if present). Consider documenting both accepted formats so
callers aren’t misled.
##########
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##########
@@ -1197,6 +1197,8 @@ class DatasetResource extends LazyLogging {
)
.head
+ val ownerNode = datasetsNode.children.get.head
+
Review Comment:
`retrieveLatestDatasetVersion` builds logical paths using `user.getEmail` as
the owner segment. For datasets the caller doesn’t own (but has read access
to), the returned file paths won’t resolve via `FileResolver` because it looks
up datasets by the *dataset owner’s* email. Use the dataset’s actual owner
email when constructing the tree (and avoid `children.get.head` while you’re
here).
##########
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala:
##########
@@ -1413,6 +1415,8 @@ class DatasetResource extends LazyLogging {
)
.head
+ val ownerFileNode = datasetsNode.children.get.head
Review Comment:
Using `datasetsNode.children.get.head` is brittle (Option.get + assumes
ordering). Prefer `getChildren.headOption.getOrElse(...)` so an unexpected
empty tree fails with a clear message instead of a
`NoSuchElementException`/`None.get`.
--
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]
