aicam commented on code in PR #6502:
URL: https://github.com/apache/texera/pull/6502#discussion_r3634361850
##########
amber/src/main/python/pytexera/storage/dataset_file_document.py:
##########
@@ -56,20 +61,28 @@ def __init__(self, file_path: str):
Parses the file path into dataset metadata.
:param file_path:
- Expected format -
"/ownerEmail/datasetName/versionName/fileRelativePath"
- Example:
"/[email protected]/twitterDataset/v1/california/irvine/tw1.csv"
+ Expected format -
+ "/datasets/ownerEmail/datasetName/versionName/fileRelativePath"
+ Example:
+ "/datasets/[email protected]/twitterDataset/v1/california/tw1.csv"
+ The leading "datasets" resource-type segment is required; a path
+ without it is rejected. Mirrors FileResolver on the backend.
"""
parts = file_path.strip("/").split("/")
- if len(parts) < 4:
+
+ # The datasets prefix is required and identifies the path as a dataset
Review Comment:
The fact that datasets prefix is required is repeated everywhere
##########
amber/src/main/python/pytexera/storage/dataset_file_document.py:
##########
@@ -24,6 +24,11 @@
class DatasetFileDocument:
+ # Leading resource-type segment on dataset logical paths. It identifies
the path as
+ # a dataset path and is REQUIRED, mirroring FileResolver on the backend. A
path
+ # without it is rejected.
+ RESOURCE_TYPE_PREFIX = "datasets"
Review Comment:
it should be an enum to be able to expand it later
##########
common/workflow-core/src/main/scala/org/apache/texera/amber/core/storage/FileResolver.scala:
##########
@@ -75,9 +75,19 @@ object FileResolver {
filePath.toUri
}
+ // The resource-type segment that identifies a dataset logical path. The
prefix selects the
+ // resource kind (and thus the backing table) during resolution, so it is
REQUIRED: a path
+ // without it is not a dataset path. Other resource types (e.g. models) get
their own prefix.
+ val DATASET_PATH_PREFIX = "datasets"
Review Comment:
ditto
##########
common/workflow-core/src/main/scala/org/apache/texera/amber/core/storage/FileResolver.scala:
##########
@@ -88,23 +98,25 @@ object FileResolver {
val filePath = Paths.get(fileName)
val pathSegments = (0 until
filePath.getNameCount).map(filePath.getName(_).toString).toArray
- if (pathSegments.length < 4) {
+ // A dataset path must start with the datasets prefix and carry at least
Review Comment:
again, repeated here that its required
##########
common/workflow-core/src/main/scala/org/apache/texera/amber/core/storage/FileResolver.scala:
##########
@@ -75,9 +75,19 @@ object FileResolver {
filePath.toUri
}
+ // The resource-type segment that identifies a dataset logical path. The
prefix selects the
+ // resource kind (and thus the backing table) during resolution, so it is
REQUIRED: a path
+ // without it is not a dataset path. Other resource types (e.g. models) get
their own prefix.
+ val DATASET_PATH_PREFIX = "datasets"
+
/**
* Parses a dataset file path and extracts its components.
- * Expected format: /ownerEmail/datasetName/versionName/fileRelativePath
+ *
+ * Expected format:
/datasets/ownerEmail/datasetName/versionName/fileRelativePath
+ *
+ * The leading [[DATASET_PATH_PREFIX]] segment is required — it identifies
the path as a
Review Comment:
This is too verbose comment
##########
frontend/src/app/workspace/component/dataset-selection-modal/dataset-selection-modal.component.ts:
##########
@@ -83,7 +83,13 @@ export class DatasetSelectionModalComponent implements
OnInit {
this.datasets = datasets;
const selectedPath = this.data.selectedPath;
if (selectedPath) {
- const [ownerEmail, datasetName, versionName] =
selectedPath.split("/").filter(part => part.length > 0);
+ const segments = selectedPath.split("/").filter(part => part.length
> 0);
+ // Drop the resource-type prefix ("datasets") if present so
owner/dataset/version
+ // line up (the stored path may or may not carry the prefix).
+ if (segments[0] === "datasets") {
Review Comment:
This is a hardcoded if condition with string "datasets", it should not
ignore prefix, frontend should follow backend
##########
file-service/src/main/scala/org/apache/texera/service/type/dataset/DatasetFileNode.scala:
##########
@@ -81,15 +83,20 @@ object DatasetFileNode {
): List[DatasetFileNode] = {
val rootNode = new DatasetFileNode("/", "directory", null, "")
+ // Add "datasets" prefix node
+ val datasetsNode =
+ new DatasetFileNode(FileResolver.DATASET_PATH_PREFIX, "directory",
rootNode, "")
Review Comment:
What does keyword "directory" mean here?
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/dataset/FileListerSourceOpExec.scala:
##########
@@ -28,13 +29,38 @@ import
org.apache.texera.dao.jooq.generated.tables.Dataset.DATASET
import
org.apache.texera.dao.jooq.generated.tables.DatasetVersion.DATASET_VERSION
import org.apache.texera.dao.jooq.generated.tables.User.USER
+object FileListerSourceOpExec {
+
+ /**
+ * Parses a dataset version path into its (ownerEmail, datasetName,
versionName) components.
+ *
+ * Expected format: /datasets/ownerEmail/datasetName/versionName
+ *
+ * The leading "datasets" resource-type prefix is required (mirroring
FileResolver); empty
Review Comment:
again!
--
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]
