apache / apache/incubator-xtable

Agent-legible CLI: structured output, real exit codes, dry-run and sync-mode

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

Description

### Feature Request / Improvement

Make the XTable CLI legible to a program rather than only to a human reading logs. This is the foundation for any agent tooling, but it is worth doing on its own: Airflow, cron, CI and monitoring wrappers all need the same thing.

## The defects it starts from

`ConversionController.sync()` returns `Map`, and each `SyncResult` already carries everything a caller needs: `mode`, `lastInstantSynced`, `syncStartTime`, `syncDuration`, `tableFormatSyncStatus` (a `SyncStatusCode` of SUCCESS, ABORTED or ERROR plus `ErrorDetails`), `recommendedSyncMode`, and a per-catalog status list.

`RunSync.syncTableMetdata` discards all of it:

```java
conversionController.sync(conversionConfig, conversionSourceProvider); // return value dropped
} catch (Exception e) {
log.error("Error running sync for {}", table.getTableBasePath(), e); // swallowed, loop continues
}
```

Consequences today:

1. The structured result never reaches the caller. Log text is the only interface.
2. Per-table failures are swallowed and never aggregated, so a run where every table failed is indistinguishable from a clean run.
3. **The exit code is always 0.** There is no `System.exit` in `RunSync`. The argument-parse failure path prints help and returns, so even a malformed command line exits 0.

Point 3 is a plain bug for any automated caller.

## Scope

- Return the aggregated results from `syncTableMetdata` instead of `void`.
- Real exit codes: `0` all targets succeeded, `1` usage or configuration error, `2` partial failure, `3` all failed. Fix the parse-failure path.
- `--output json|text`, defaulting to `text` so existing behaviour is unchanged. The JSON is close to a direct serialization of `SyncResult`: one record per table and target, with status, sync mode, last instant synced, duration, recommended sync mode, and error details.
- `--dryRun`: validate the config, resolve source and targets, report what would be synced, write nothing. This is also the guardrail the MCP `convert` tool will need.
- `--syncMode FULL|INCREMENTAL`. `RunSync` currently hardcodes `INCREMENTAL`, even though `SyncResult.recommendedSyncMode` already tells the caller when a full sync is needed. There is no way to act on it from the CLI today.
- Report a no-op explicitly. `lastInstantSynced` makes it detectable, so callers should not have to infer it. PR #829 currently detects no-op syncs heuristically, which is the symptom this fixes.
- Apply the same treatment to `RunCatalogSync`.

## Out of scope

Anything agent-specific. This issue only changes what the CLI reports and what it returns to the shell.

### Are you willing to submit PR?

- [x] Yes I am willing to submit a PR!

### Code of Conduct

- [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by reading `RunSync` and `RunCatalogSync`, then trace how `ConversionController.sync()` returns `SyncResult` and how `RunSync.syncTableMetdata` currently handles it. Check the CLI argument parsing and existing sync tests before mapping each requested behavior to coverage. Done means both commands report structured results and no-ops, honor the requested modes and dry-run, and return the specified exit codes.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
cli, data-engineering
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.