vinishjail97 commented on code in PR #829:
URL: https://github.com/apache/incubator-xtable/pull/829#discussion_r3439807437
##########
.claude/settings.json:
##########
@@ -0,0 +1,11 @@
+{
+ "permissions": {
+ "allow": [
+ "Bash(mkdir -p /tmp/xtable_run*)",
+ "Bash(python3 .claude/skills/xtable-converter/scripts/*)",
+ "Bash(java -cp *)",
Review Comment:
`Bash(java -cp *)` auto-approves running **any** Java main class with
**any** classpath and args, with no prompt, for every contributor who clones
the repo — that is effectively arbitrary-code-execution approval. Scope this
down to the exact invocation the jol-core workaround needs (a specific main
class / jar pattern), or drop it and let it prompt.
##########
.claude/skills/xtable-converter/scripts/parse_result.py:
##########
@@ -0,0 +1,132 @@
+#!/usr/bin/env python3
+"""Parse the XTable run log into per-(table, target) outcomes.
+
+Reads run_meta.json (from run_xtable.sh) and the datasetConfig YAML, scans the
+log, and writes log_results.json next to run_meta.json.
+
+log_status per (table, target): "ok" | "error" | "unknown".
+"unknown" is fine — verify.py's storage check is the ground truth.
+
+If the wording of your XTable version's logs differs, pin the patterns below
+(see references/troubleshooting.md, "Pinning log markers").
+"""
+import argparse
+import json
+import os
+import re
+import sys
+
+# Generic markers; pin to your XTable version for sharper "ok" detection.
+SUCCESS_PATTERNS = [
+ r"completed sync",
+ r"sync completed",
+ r"completed successfully",
+ r"running sync .* completed",
+]
+ERROR_PATTERNS = [
+ r"\bERROR\b",
+ r"\bException\b",
+ r"\bFAILED\b",
+ r"Error running sync",
+]
+
+
+def parse_config(path):
+ """Minimal YAML reader for the flat datasetConfig structure (no deps)."""
+ targets, tables = [], []
+ section = None
+ current = None
+ with open(path) as f:
+ for raw in f:
+ line = raw.rstrip("\n")
+ s = line.strip()
+ if not s or s.startswith("#"):
+ continue
+ if s.startswith("targetFormats:"):
+ section = "targets"; continue
+ if s.startswith("datasets:"):
+ section = "datasets"; continue
+ if s.startswith("sourceFormat:"):
+ section = None; continue
+ if section == "targets" and s.startswith("- "):
+ targets.append(s[2:].strip()); continue
+ if section == "datasets":
+ if s.startswith("- "):
+ current = {}; tables.append(current)
+ s = s[2:].strip()
+ if ":" in s and current is not None:
+ k, v = s.split(":", 1)
+ current[k.strip()] = v.strip()
+ names = [t.get("tableName") or t.get("tableBasePath",
"?").rstrip("/").rsplit("/", 1)[-1] for t in tables]
+ return targets, tables, names
+
+
+def main():
+ ap = argparse.ArgumentParser()
+ ap.add_argument("run_meta")
+ ap.add_argument("--config", required=True)
+ args = ap.parse_args()
+
+ with open(args.run_meta) as f:
+ meta = json.load(f)
+ targets, tables, names = parse_config(args.config)
+ out_path = os.path.join(os.path.dirname(os.path.abspath(args.run_meta)),
"log_results.json")
+
+ results = []
+
+ def emit():
+ with open(out_path, "w") as f:
+ json.dump({"run_meta": meta, "results": results}, f, indent=2)
+ print(f"Wrote {out_path}")
+ for r in results:
+ print(f" {r['table']:<24} -> {r['target']:<8}
log_status={r['log_status']}"
+ + (f" ({r['message'][:100]})" if r.get("message") else ""))
+
+ log_path = meta.get("log")
+ if not log_path or not os.path.exists(log_path):
+ msg = meta.get("error") or f"log file missing
(exit_code={meta.get('exit_code')})"
+ for name in names or ["<all>"]:
+ for tgt in targets or ["<all>"]:
+ results.append({"table": name, "target": tgt, "log_status":
"error", "message": msg})
+ emit(); return
+
+ with open(log_path, errors="replace") as f:
+ lines = f.readlines()
+
+ success_re = re.compile("|".join(SUCCESS_PATTERNS), re.IGNORECASE)
+ error_re = re.compile("|".join(ERROR_PATTERNS))
+ process_failed = meta.get("exit_code", 1) != 0
+
+ for name, tbl in zip(names, tables):
+ ident_bits = [b for b in (name, tbl.get("tableBasePath")) if b]
+ # Lines mentioning this table (by name or base path)
+ mentions = [(i, ln) for i, ln in enumerate(lines) if any(b in ln for b
in ident_bits)]
+ for tgt in targets:
+ status, message = "unknown", None
+ # error lines mentioning the table, or table mentions inside a
stack-trace window
+ for i, ln in mentions:
Review Comment:
Error detection isn't target-scoped. `mentions` is every line that names the
table (or its basePath), so this `error` branch fires for **all** targets of a
table as soon as any one of them logs an error. The success branch below
correctly gates on `tgt.lower() in ln.lower()`, but this one doesn't. Combined
with verify.py — which short-circuits `log_status == "error"` straight to
FAILED and never runs the storage check — a single failed target produces false
FAILED verdicts for sibling targets that actually succeeded. Scope the error
match to the target the same way the success match is scoped.
##########
.claude/skills/xtable-converter/SKILL.md:
##########
@@ -0,0 +1,147 @@
+---
+name: xtable-converter
+description: Convert lakehouse tables between Apache Hudi, Apache Iceberg, and
Delta Lake formats using Apache XTable (incubator-xtable) driven by natural
language instead of hand-written YAML. Use this skill whenever the user asks to
convert, sync, translate, or expose a table in another format — e.g. "convert
my Hudi table to Iceberg", "make this Delta table queryable from Snowflake",
"sync s3://bucket/table to Delta and Iceberg", "run xtable on these tables" —
or mentions XTable, table format interoperability, or datasetConfig YAML. Every
conversion MUST end with an explicit per-table SUCCESS / FAILED / UNVERIFIED
verdict; never report an implied or assumed success.
+---
+
+# XTable Converter
+
+Turn a natural-language request like *"convert my Hudi table at
s3://lake/orders to Iceberg"* into a validated XTable `datasetConfig` YAML, run
the conversion, and report a verified per-table verdict. XTable converts
**metadata only** — it never copies or rewrites data files, so conversions are
cheap and non-destructive.
+
+## Non-negotiable rules
+
+1. **No silent failures.** Every run ends with the Conversion Report (format
below) showing SUCCESS, FAILED, or UNVERIFIED per (table, target format) pair.
The XTable jar catches per-table exceptions and can exit 0 even when a table
failed — exit code alone is NEVER proof of success.
+2. **No guessed paths or credentials.** If the user hasn't given a table path,
source format, or required partition spec, ask. Never invent S3/ADLS/GCS paths
or credentials.
+3. **Run immediately when slots are complete.** If the request contains all
required slots, write the YAML and run without asking for confirmation — show
the config inline as part of reporting, not as a blocker. Only pause when a
required slot is missing or ambiguous.
+4. **SUCCESS requires ground-truth verification.** "The log said it worked" is
only UNVERIFIED until target metadata is confirmed on storage.
+
+## Workflow
+
+### Step 1 — Extract conversion slots from the user's request
+
+| Slot | Required? | Notes |
+|---|---|---|
+| `sourceFormat` | Yes | One of HUDI, ICEBERG, DELTA. Infer from phrases like
"my Hudi table". Ask if ambiguous. |
+| `targetFormats` | Yes | One or more of the other two formats. |
+| `tableBasePath` | Yes (per table) | s3://, gs://, abfs://, hdfs://, or local
file path. Never guess. |
+| `tableName` | Yes (per table) | Default to the last path segment if user
doesn't name it; mention the assumption. |
+| `tableDataPath` | Required for ICEBERG sources | Usually `<basePath>/data`.
Optional otherwise. |
+| `partitionSpec` | Required for partitioned HUDI sources | Format
`path:type[:format]`, comma-separated for multi-partition. See
`references/config-schema.md`. |
+| `namespace` | Optional | Only when syncing to a catalog. |
+| Catalog / hadoop / converter configs | Optional | `--icebergCatalogConfig`,
`--hadoopConfig`, `--convertersConfig`. See `references/config-schema.md`. |
+
+If the source is HUDI and you don't know whether the table is partitioned,
ask: "Is the table partitioned? If so, which column(s) and are they
string/date-string values or numeric timestamps?" — a wrong partitionSpec is
the #1 cause of failed Hudi conversions. Use `VALUE` for string/date-string
partition columns (identity transform); use `DAY`/`MONTH`/`YEAR`/`HOUR` only
for numeric timestamp or long epoch columns.
+
+### Step 2 — Build and validate the YAML, then write it to disk
+
+Construct the config directly. Validate before writing:
+- `sourceFormat` and each entry in `targetFormats` must be HUDI, ICEBERG, or
DELTA (uppercase).
+- `targetFormats` must not include `sourceFormat`, and must have no duplicates.
+- Each dataset must have `tableBasePath` (valid scheme) and `tableName`.
+- ICEBERG sources must have `tableDataPath`.
+- `partitionSpec` is only valid for HUDI sources; each entry must be
`path:type` or `path:type:format`, where type is VALUE, YEAR, MONTH, DAY, or
HOUR, and date types require a format string.
+
+If any slot is invalid, report the reason and ask the user to clarify — do not
write a bad config.
+
+Example output for *"convert my partitioned Hudi table at /tmp/orders to
Iceberg and Delta"* where `order_date` is a string column:
+
+```yaml
+sourceFormat: HUDI
+targetFormats:
+ - ICEBERG
+ - DELTA
+datasets:
+ - tableBasePath: /tmp/orders
+ tableName: orders
+ partitionSpec: order_date:VALUE
+```
+
+Write it to disk immediately — do not wait for confirmation:
+
+```
+RUN_DIR=/tmp/xtable_run
+mkdir -p $RUN_DIR
+# write config.yaml to $RUN_DIR/config.yaml
+```
+
+Show the YAML in the same message that reports the conversion result.
+
+### Step 3 — Run the conversion
+
+```bash
+bash scripts/run_xtable.sh /tmp/xtable_run/config.yaml \
Review Comment:
This invokes `bash scripts/run_xtable.sh` (relative to the skill dir), but
the committed allow rule is `Bash(bash
.claude/skills/xtable-converter/scripts/*)`. The two won't match, so this will
still prompt. Make the documented command and the permission pattern use the
same path.
##########
.claude/settings.json:
##########
@@ -0,0 +1,11 @@
+{
Review Comment:
Scope/governance question before anything else: this commits
Claude-Code-specific tooling **and** a team-wide permission allowlist into a
vendor-neutral ASF incubator repo. Everyone who clones and runs Claude Code
here inherits these auto-approve rules. Has this been raised on dev@ / with the
PPMC? At minimum, consider whether the skill belongs in-repo vs. a separate
tools repo, and whether committed permissions are appropriate for an Apache
project (vs. each contributor opting in via the git-ignored
`settings.local.json`).
##########
.claude/skills/xtable-converter/scripts/verify.py:
##########
@@ -0,0 +1,254 @@
+#!/usr/bin/env python3
+"""Verify target metadata on storage and merge with log results into the final
verdict.
+
+Per (table, target), checks that the expected metadata exists at tableBasePath
+AND is newer than the run start timestamp:
+ DELTA -> _delta_log/*.json
+ ICEBERG -> metadata/*.metadata.json
+ HUDI -> .hoodie timeline files (.commit/.deltacommit/.replacecommit,
incl. .hoodie/timeline/)
+
+Local paths: checked directly. s3:// or s3a://: aws CLI. Other schemes: hadoop
fs.
+If no tooling/access -> storage_status = "not_possible" -> verdict UNVERIFIED.
+
+Writes final_report.json with verdicts: SUCCESS | FAILED | UNVERIFIED.
+Exit code: 0 if all SUCCESS, 3 if any UNVERIFIED (and none failed), 1 if any
FAILED.
+"""
+import argparse
+import glob
+import json
+import os
+import re
+import shutil
+import subprocess
+import sys
+
+METADATA_CHECKS = {
+ "DELTA": {"subdir": "_delta_log", "pattern": r"\.json$"},
+ "ICEBERG": {"subdir": "metadata", "pattern": r"\.metadata\.json$"},
+ "HUDI": {"subdir": ".hoodie", "pattern":
r"(\.commit|\.deltacommit|\.replacecommit|hoodie\.properties)$"},
+}
+
+# Patterns used to detect new commits on the SOURCE side (local paths only).
+SOURCE_COMMIT_CHECKS = {
+ "HUDI": {"subdir": ".hoodie", "pattern":
r"\.(commit|deltacommit|replacecommit)$"},
+ "DELTA": {"subdir": "_delta_log", "pattern": r"\.json$"},
+}
+
+
+def run_cmd(cmd):
+ try:
+ p = subprocess.run(cmd, capture_output=True, text=True, timeout=120)
+ return p.returncode, p.stdout, p.stderr
+ except FileNotFoundError:
+ return None, "", "tool not found"
+ except subprocess.TimeoutExpired:
+ return 124, "", "timeout"
+
+
+def check_local(base, target, start_epoch):
+ cfg = METADATA_CHECKS[target]
+ base = re.sub(r"^file://", "", base)
+ d = os.path.join(base, cfg["subdir"])
+ if not os.path.isdir(d):
+ return "missing", f"{d} does not exist"
+ pat = re.compile(cfg["pattern"])
+ files = [f for f in glob.glob(os.path.join(d, "**", "*"), recursive=True)
+ if os.path.isfile(f) and pat.search(os.path.basename(f))]
+ if not files:
+ return "missing", f"no matching metadata files under {d}"
+ newest = max(files, key=os.path.getmtime)
+ mtime = os.path.getmtime(newest)
+ if mtime >= start_epoch:
+ return "fresh", f"{os.path.relpath(newest, base)} (mtime {int(mtime)})"
+ return "stale", f"newest is {os.path.relpath(newest, base)}, older than
run start"
+
+
+def check_s3(base, target, start_epoch):
+ if not shutil.which("aws"):
+ return "not_possible", "aws CLI not available"
+ cfg = METADATA_CHECKS[target]
+ url = re.sub(r"^s3a://", "s3://", base).rstrip("/") + "/" + cfg["subdir"]
+ "/"
+ rc, out, err = run_cmd(["aws", "s3", "ls", url, "--recursive"])
+ if rc is None or rc != 0:
+ return "not_possible", f"aws s3 ls failed: {err.strip()[:160] or 'rc='
+ str(rc)}"
+ pat = re.compile(cfg["pattern"])
+ newest = None # (date_str, key)
+ for line in out.splitlines():
+ parts = line.split()
+ if len(parts) >= 4 and pat.search(parts[-1]):
+ stamp = parts[0] + " " + parts[1]
+ if newest is None or stamp > newest[0]:
+ newest = (stamp, parts[-1])
+ if not newest:
+ return "missing", f"no matching metadata at {url}"
+ import datetime
+ try:
+ ts = datetime.datetime.strptime(newest[0], "%Y-%m-%d
%H:%M:%S").timestamp()
Review Comment:
`datetime.strptime(...).timestamp()` interprets the naive datetime in the
host's **local** timezone, but `aws s3 ls` prints timestamps in **UTC**. On any
non-UTC machine the freshness comparison is wrong by the tz offset, yielding
spurious `stale`/`fresh`. Use `calendar.timegm(struct_time)` (or attach
`tzinfo=timezone.utc`) so the epoch is computed as UTC.
##########
.claude/skills/xtable-converter/scripts/verify.py:
##########
@@ -0,0 +1,254 @@
+#!/usr/bin/env python3
+"""Verify target metadata on storage and merge with log results into the final
verdict.
+
+Per (table, target), checks that the expected metadata exists at tableBasePath
+AND is newer than the run start timestamp:
+ DELTA -> _delta_log/*.json
+ ICEBERG -> metadata/*.metadata.json
+ HUDI -> .hoodie timeline files (.commit/.deltacommit/.replacecommit,
incl. .hoodie/timeline/)
+
+Local paths: checked directly. s3:// or s3a://: aws CLI. Other schemes: hadoop
fs.
+If no tooling/access -> storage_status = "not_possible" -> verdict UNVERIFIED.
+
+Writes final_report.json with verdicts: SUCCESS | FAILED | UNVERIFIED.
+Exit code: 0 if all SUCCESS, 3 if any UNVERIFIED (and none failed), 1 if any
FAILED.
+"""
+import argparse
+import glob
+import json
+import os
+import re
+import shutil
+import subprocess
+import sys
+
+METADATA_CHECKS = {
+ "DELTA": {"subdir": "_delta_log", "pattern": r"\.json$"},
+ "ICEBERG": {"subdir": "metadata", "pattern": r"\.metadata\.json$"},
+ "HUDI": {"subdir": ".hoodie", "pattern":
r"(\.commit|\.deltacommit|\.replacecommit|hoodie\.properties)$"},
Review Comment:
The HUDI freshness pattern includes `hoodie.properties`, which is written
once at table creation and not rewritten per sync. Since freshness is
`max(mtime)` over matches, a present-but-old timeline plus a
`hoodie.properties` from creation could still read as the newest file and skew
the check. Restrict the freshness match to timeline instants
(`*.commit`/`*.deltacommit`/`*.replacecommit`) and keep `hoodie.properties`
only for the existence check if needed.
##########
.claude/skills/xtable-converter/SKILL.md:
##########
@@ -0,0 +1,147 @@
+---
+name: xtable-converter
+description: Convert lakehouse tables between Apache Hudi, Apache Iceberg, and
Delta Lake formats using Apache XTable (incubator-xtable) driven by natural
language instead of hand-written YAML. Use this skill whenever the user asks to
convert, sync, translate, or expose a table in another format — e.g. "convert
my Hudi table to Iceberg", "make this Delta table queryable from Snowflake",
"sync s3://bucket/table to Delta and Iceberg", "run xtable on these tables" —
or mentions XTable, table format interoperability, or datasetConfig YAML. Every
conversion MUST end with an explicit per-table SUCCESS / FAILED / UNVERIFIED
verdict; never report an implied or assumed success.
+---
+
+# XTable Converter
+
+Turn a natural-language request like *"convert my Hudi table at
s3://lake/orders to Iceberg"* into a validated XTable `datasetConfig` YAML, run
the conversion, and report a verified per-table verdict. XTable converts
**metadata only** — it never copies or rewrites data files, so conversions are
cheap and non-destructive.
+
+## Non-negotiable rules
+
+1. **No silent failures.** Every run ends with the Conversion Report (format
below) showing SUCCESS, FAILED, or UNVERIFIED per (table, target format) pair.
The XTable jar catches per-table exceptions and can exit 0 even when a table
failed — exit code alone is NEVER proof of success.
+2. **No guessed paths or credentials.** If the user hasn't given a table path,
source format, or required partition spec, ask. Never invent S3/ADLS/GCS paths
or credentials.
+3. **Run immediately when slots are complete.** If the request contains all
required slots, write the YAML and run without asking for confirmation — show
the config inline as part of reporting, not as a blocker. Only pause when a
required slot is missing or ambiguous.
+4. **SUCCESS requires ground-truth verification.** "The log said it worked" is
only UNVERIFIED until target metadata is confirmed on storage.
+
+## Workflow
+
+### Step 1 — Extract conversion slots from the user's request
+
+| Slot | Required? | Notes |
+|---|---|---|
+| `sourceFormat` | Yes | One of HUDI, ICEBERG, DELTA. Infer from phrases like
"my Hudi table". Ask if ambiguous. |
+| `targetFormats` | Yes | One or more of the other two formats. |
+| `tableBasePath` | Yes (per table) | s3://, gs://, abfs://, hdfs://, or local
file path. Never guess. |
+| `tableName` | Yes (per table) | Default to the last path segment if user
doesn't name it; mention the assumption. |
+| `tableDataPath` | Required for ICEBERG sources | Usually `<basePath>/data`.
Optional otherwise. |
+| `partitionSpec` | Required for partitioned HUDI sources | Format
`path:type[:format]`, comma-separated for multi-partition. See
`references/config-schema.md`. |
+| `namespace` | Optional | Only when syncing to a catalog. |
+| Catalog / hadoop / converter configs | Optional | `--icebergCatalogConfig`,
`--hadoopConfig`, `--convertersConfig`. See `references/config-schema.md`. |
+
+If the source is HUDI and you don't know whether the table is partitioned,
ask: "Is the table partitioned? If so, which column(s) and are they
string/date-string values or numeric timestamps?" — a wrong partitionSpec is
the #1 cause of failed Hudi conversions. Use `VALUE` for string/date-string
partition columns (identity transform); use `DAY`/`MONTH`/`YEAR`/`HOUR` only
for numeric timestamp or long epoch columns.
+
+### Step 2 — Build and validate the YAML, then write it to disk
+
+Construct the config directly. Validate before writing:
+- `sourceFormat` and each entry in `targetFormats` must be HUDI, ICEBERG, or
DELTA (uppercase).
+- `targetFormats` must not include `sourceFormat`, and must have no duplicates.
+- Each dataset must have `tableBasePath` (valid scheme) and `tableName`.
+- ICEBERG sources must have `tableDataPath`.
+- `partitionSpec` is only valid for HUDI sources; each entry must be
`path:type` or `path:type:format`, where type is VALUE, YEAR, MONTH, DAY, or
HOUR, and date types require a format string.
+
+If any slot is invalid, report the reason and ask the user to clarify — do not
write a bad config.
+
+Example output for *"convert my partitioned Hudi table at /tmp/orders to
Iceberg and Delta"* where `order_date` is a string column:
+
+```yaml
+sourceFormat: HUDI
+targetFormats:
+ - ICEBERG
+ - DELTA
+datasets:
+ - tableBasePath: /tmp/orders
+ tableName: orders
+ partitionSpec: order_date:VALUE
+```
+
+Write it to disk immediately — do not wait for confirmation:
+
+```
+RUN_DIR=/tmp/xtable_run
Review Comment:
`RUN_DIR=/tmp/xtable_run` is a fixed path reused on every invocation, so
`run_meta.json` and the run logs are overwritten each run and two concurrent
conversions race on the same files. Use a per-run dir (`mktemp -d
/tmp/xtable_run.XXXXXX`) and pass it through.
--
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]
