deepakpanda93 opened a new pull request, #19747:
URL: https://github.com/apache/hudi/pull/19747
### Describe the issue this Pull Request addresses
Closes #15888 (HUDI-6042).
`SparkClientFunctionalTestHarness#basePath()` returns a scheme-qualified
path, unlike `HoodieCommonTestHarness`, `CLIFunctionalTestHarness` and the
examples harness, which all return an unqualified one.
The scheme breaks any helper that passes the value to `java.nio.file.Paths`:
```
Paths.get("file:///var/.../dataset", "2016/03/15") ->
file:/var/.../dataset/2016/03/15
isAbsolute = false
```
`file:` is read as an ordinary directory name, so the result is a
**relative** path. `FileCreateUtils#createPartitionMetaFile` (line 398) and
`createMarkerFile` (line 448) hand the raw string to `Paths.get`, so they wrote
under the working directory instead of the table, without failing. Lines 384
and 409 of the same class normalise via `getBasePath().toUri().getPath()` and
were unaffected, which is why this stayed hidden.
`TestSparkSampleWritesUtils` carried a `basePath()` override with a `TODO
remove this and fix parent class (HUDI-6042)` comment, working around exactly
this.
### Summary and Changelog
`basePath()` now returns the unqualified path; a new `baseUri()` returns the
URI form for callers that want the scheme. This aligns the harness with the
three other test harnesses in the repo.
- `SparkClientFunctionalTestHarness`: `basePath()` returns
`tempDir.toAbsolutePath().toString()`; added `baseUri()`.
- Removed 20 workarounds that existed only because of this bug:
- `URI.create(basePath()).getPath()` x18 in `TestRollingMetadata`,
`TestSparkRDDWriteClient`, `TestUpgradeDowngrade`
- `basePath().substring(7)` in `TestHoodieSparkRollback`
- the `HUDI-6042` override in `TestSparkSampleWritesUtils`
- Updated call sites that consumed the scheme implicitly:
- `TestSparkRDDWriteClient`: `basePath + "_UTC"` -> `"/_UTC"`. `toUri()`
appends a trailing slash for an existing directory and `toString()` does not,
so this had become a sibling directory escaping `@TempDir` cleanup.
- `TestHoodieSparkRollback` (x2): `basePath + ".hoodie/"` ->
`"/.hoodie/"`, same trailing-slash reason.
- `TestHoodieSparkRollback` (x3):
`metaClient.getBasePath().toString().substring(5)` ->
`getBasePath().toUri().getPath()`. The old form stripped a `file:` prefix that
no longer exists; the new form yields the path component whether or not a
scheme is present, matching the idiom already used in `FileCreateUtils`.
- Added `TestSparkClientFunctionalTestHarness` covering the path contract.
### Impact
Test infrastructure only. No production code is touched. Callers that need
the scheme use `baseUri()`.
### Risk Level
low
Aligns this harness with `HoodieCommonTestHarness`,
`CLIFunctionalTestHarness` and the examples harness, all of which already
return unqualified paths. The change is toward the more permissive form: Hadoop
and `StoragePath` accept both qualified and unqualified paths, while
`java.nio.file.Paths` accepts only unqualified.
Verification:
| Suite | Result |
| --- | --- |
| `TestSparkClientFunctionalTestHarness` (new) | 4/4 |
| `TestSparkRDDWriteClient` | 19/19 |
| `TestRollingMetadata` | 10/10 |
| `TestUpgradeDowngrade` | 52/52 |
| `TestSparkSampleWritesUtils` | 3/3, override deleted |
| `TestHoodieSparkCopyOnWriteTableRollbackTableVersionSix` | 4/4 |
| `TestHoodieSparkMergeOnReadTableRollback` | 26/26 |
Negative control: reverting only the harness body while keeping the new
tests fails 3 of the 4, including the end-to-end case asserting partition
metadata lands under the table directory. The fourth (storage resolution)
passes either way and is a regression guard rather than a bug detector.
The two rollback classes were also run against an all-master baseline
(source and installed artifacts both at master) and pass 30/30 on both sides,
confirming no regression.
Since the compiler cannot catch a change of `String` content, all 77 direct
and transitive subclasses of the harness were swept for every way a caller
could depend on the scheme: `substring` on a base path,
`replace`/`startsWith`/`contains("file:")`, `split`/`indexOf` on a colon,
`URI`/`URL` construction, `length()` arithmetic, regex, and concatenation
without a separator. No remaining dependencies. `TestGcsEventsHoodieIncrSource`
overrides `basePath()` to return the qualified form (pre-existing, from
HUDI-4850) and so is unaffected.
### Documentation Update
none
### Contributor's checklist
- [x] Read through [contributor's
guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Change Logs and Impact were stated clearly
- [x] Adequate tests were added if applicable
- [x] CI passed
--
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]
