aglinxinyuan opened a new pull request, #7036:
URL: https://github.com/apache/texera/pull/7036
### What changes were proposed in this PR?
`DatasetFileNode` has two tree builders; only one is reachable. This removes
the dead one.
| Builder | Callers | Action |
| --- | --- | --- |
| `fromLakeFSRepositoryCommittedObjects` | live — the dataset REST resources
| untouched |
| `fromPhysicalFileNodes` + private `addNodeToTree` | none outside the spec
| **removed** |
The dead builder converts a `PhysicalFileNode` tree — the pre-LakeFS,
local-JGit representation — into a `DatasetFileNode` tree. Since dataset
storage moved to LakeFS, nothing produces that input in production.
Also removed, because they become unused:
- `import
org.apache.texera.amber.core.storage.util.dataset.PhysicalFileNode` and `import
java.util` in `DatasetFileNode.scala`
- `import PhysicalFileNode`, `import java.nio.file.{Files, Path}` and
`import scala.jdk.CollectionConverters._` in the spec
- the `fromPhysicalFileNodes` spec block, and the mention of it in the
spec's header comment
Leaving any of those behind would fail `scalafixAll --check`.
Side effect worth noting: the two builders shared ~24 lines of
byte-identical owner/dataset/version scaffolding plus two byte-identical local
`sortChildren` definitions. Deleting the dead one removes that duplication
without needing a refactor.
−141 lines, no behaviour change.
**Follow-up, deliberately not in this PR.** This drops file-service's only
compile-time dependency on `PhysicalFileNode`. Once it lands,
`PhysicalFileNode` together with `JGitVersionControl.getRootFileNodeOfCommit` /
`createOrGetNode` / `ensureParentChildLink` and
`GitVersionControlLocalFileStorage.retrieveRootFileNodesOfVersion` has no
non-test caller anywhere. That chain spans `common/workflow-core` and the amber
test tree, so it deserves its own review rather than being bundled here.
### Any related issues, documentation, discussions?
Closes #7035
### How was this PR tested?
Existing tests only — this PR adds none, since it removes code and the spec
case that covered it.
Locally, from the repo root with Java 17:
- `sbt "scalafixAll --check"` — clean (this is what catches the five
now-unused imports).
- `sbt scalafmtCheckAll` — clean.
- `sbt "FileService/testOnly *DatasetFileNodeSpec"` — 8 tests, all pass (was
9; the removed one covered `fromPhysicalFileNodes`).
Verification that nothing references the removed code, re-runnable by a
reviewer:
```
git grep -nw "fromPhysicalFileNodes\|addNodeToTree"
```
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)
--
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]
