CalvinKirs commented on PR #67986:
URL: https://github.com/apache/doris/pull/67986#issuecomment-5678079944
<!-- doris-repo-review:v1:begin -->
### Local pipeline review — ✅ PASS
```yaml
schema: doris-repo-review/v1
status: PASS
pr: apache/doris#67986
commit: 858d63e9e9f02dbb5abe1306ff952c92cfae7009
base: 1407093484abee7afdd318b62d7fef784e53cdd8
reviewed_at: 2026-09-15T17:43:12+08:00
reviewer: CalvinKirs
model: claude-fable-5-1
effort: xhigh
findings: {blocker: 0, major: 0, minor: 0, nit: 1}
rounds: 1
converged: true
```
**Scope**
Test-only change: one method in `OssHdfsPropertiesSpiParityTest` renamed and
its expectation updated to the URI qualification introduced by #67545. No
production code touched. `OssHdfsProperties` at the PR head is byte-identical
to `master`.
**Verification against the production code**
Each assertion was traced through
`OssHdfsProperties.validateAndNormalizeUri` / `extractBucket` /
`extractEndpointHost`:
- `oss://mybucket/x` → authority `mybucket` does not match
`OSS_QUALIFIED_AUTHORITY_PATTERN`, so it is kept as the bucket and the bound
endpoint host `cn-hangzhou.oss-dls.aliyuncs.com` is appended →
`oss://mybucket.cn-hangzhou.oss-dls.aliyuncs.com/x`. Matches the new
expectation.
- `oss://mybucket.cn-hangzhou.oss-dls.aliyuncs.com/x` → the pattern captures
`mybucket`, the same endpoint host is re-appended → output equals input. The
idempotence assertion is correct.
- `hdfs://ns1/x` → scheme check throws `IllegalArgumentException` before any
authority handling. Unchanged.
The sibling `OssHdfsPropertiesTest` (updated by #67545) already covers the
same qualification plus the native-OSS-suffix and mixed-case paths, so after
this PR both test classes agree with the single production implementation.
**Checkpoints (Part 1.3)**
- Goal / test proof: goal is to make `fe-filesystem-oss-hdfs` green again on
master; the updated test pins the current behavior and FE UT passes at the PR
head.
- Small, clear, focused: yes — single file, single test method, +8/−2.
- Concurrency: none involved.
- Lifecycle / static init: none.
- Configuration items: none added.
- Incompatible changes / rolling upgrade: none.
- Functionally parallel paths: `OssHdfsPropertiesTest` was already aligned
by #67545; no other test asserts the old passthrough.
- Special conditional checks: none added.
- Test coverage: adequate for the stated scope; negative case (non-`oss`
scheme) retained.
- Test results (.out) added/modified: none.
- Observability: n/a.
- Transaction / persistence / EditLog: n/a.
- Data writes: n/a.
- FE↔BE transported variables: n/a.
- Performance: n/a.
**Notes for maintainers**
- nit: the class Javadoc of `OssHdfsPropertiesSpiParityTest` and the comment
on `validateAndNormalizeUri` still name fe-core's `OSSHdfsPropertiesParityTest`
as the oracle, which per the PR description no longer exists on master.
Harmless; fine to clean up separately.
- The `check_coverage_fe` failure is the expected outcome for a test-only
diff and is not a defect of this PR.
<!-- doris-repo-review:v1:end -->
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
