apache / apache/incubator-xtable

A request that follows the published REST spec fails: source-data-path is required but undocumented

Open
#916 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
1.2k
Forks
212
Avg merge
4d 9h
Merged PRs (30d)
16

Description

## Describe the bug

`spec/rest-service-open-api.yaml` does not mention `source-data-path` anywhere. Its
`ConvertTableRequest` schema lists five properties and marks four required:

required: [source-format, source-table-name, source-table-path, target-formats]

`ConvertTableRequest.java:43` has the field, and `ConversionService.convertTable` dereferences it
twice with no fallback:

ConversionService.java:205 .dataPath(convertTableRequest.getSourceDataPath())
ConversionService.java:216 .basePath(convertTableRequest.getSourceDataPath())

The first is safe -- `SourceTable.java:44` defaults `dataPath` to `basePath` when null. The second is
not. It feeds `TargetTable.basePath`, and `ExternalTable.java:56` declares that parameter `@NonNull`,
so Lombok throws before any conversion starts.

The result: a client that builds its request from the spec, sending exactly the four documented
required fields, gets a failure rather than a conversion.

## Why it survived

Every test that reaches `convertTable` sets the field -- `TestConversionService` in three places and
`ITConversionService.java:381`. There is no case covering its absence, so nothing exercises the path
a spec-driven client actually takes.

## Expected behaviour

Either is defensible, and it is a maintainer call which:

1. **Document it as required.** Add `source-data-path` to the schema's properties and to `required`.
Smallest change, matches the service as written.
2. **Default it in the service.** Fall back to `source-table-path` for the target's base path, the
way `SourceTable` already falls back for the source's. This keeps the four-field request in the
spec working, and the inline comment at `ConversionService.java:215` ("set the metadata path to
the data path as the default (required by Hudi)") suggests a default was the intent.

I have a PR ready for option 1, since it is the one a spec-only change can make, and I would rather
leave `ConversionService` to someone closer to it. If you prefer option 2, close the PR and this
issue still stands as the report.

## Environment

Reproduced by reading `main` at 31ac06f; not run against a deployed service.

---
*This issue was created with AI assistance.*

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by comparing spec/rest-service-open-api.yaml with ConvertTableRequest.java and the cited ConversionService.java, SourceTable.java, and ExternalTable.java paths. Review TestConversionService and ITConversionService.java:381, then confirm with a maintainer which documented behavior is intended. Done means the chosen behavior is represented in the REST contract and covered by a test for a request without source-data-path.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, openapi
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.