-
Notifications
You must be signed in to change notification settings - Fork 1
cowork: schema command error-path hardening + honest sample footer #49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f4cd6b7
a54959e
a5e73c5
75d313d
a158bfc
1dbb981
b12a21a
35ab770
eee7094
9fbb324
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -399,6 +399,12 @@ def write_stream(self, rows: RowStream, path: str | Path) -> int: | |
|
|
||
| rows_list = list(rows) | ||
| if not rows_list: | ||
| # Zero-in is legitimate, but the output file must still exist: | ||
| # write a valid empty Avro container (record with no fields) | ||
| # instead of silently producing no artifact. | ||
| empty_schema = {"type": "record", "name": "Record", "fields": []} | ||
| with open(path, "wb") as f: | ||
| fastavro.writer(f, empty_schema, []) | ||
| return 0 | ||
|
|
||
| # Infer schema across all rows for proper type detection | ||
|
|
@@ -594,6 +600,21 @@ def _counting(stream: RowStream) -> RowStream: | |
| return result | ||
|
|
||
|
|
||
| @dataclass | ||
| class BatchConversionResult: | ||
| """Outcome of a batch conversion, including files that were skipped. | ||
|
|
||
| ``skipped`` records every file that matched ``pattern`` but was NOT | ||
| converted because its detected format differed from ``input_format`` | ||
| (or the format could not be detected at all). Surfacing these prevents | ||
| the classic silent failure where a directory conversion quietly drops | ||
| mislabeled or foreign-format files while reporting success. | ||
| """ | ||
|
|
||
| results: list[ConversionResult] = field(default_factory=list) | ||
| skipped: list[dict[str, str]] = field(default_factory=list) | ||
|
|
||
|
|
||
| def convert_batch( | ||
| input_dir: str | Path, | ||
| output_dir: str | Path, | ||
|
|
@@ -602,19 +623,26 @@ def convert_batch( | |
| pattern: str = "*", | ||
| recursive: bool = False, | ||
| **writer_kwargs: Any, | ||
| ) -> list[ConversionResult]: | ||
| ) -> BatchConversionResult: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎. |
||
| """Convert all matching files in a directory.""" | ||
| input_dir = Path(input_dir) | ||
| output_dir = Path(output_dir) | ||
| output_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| glob_pattern = f"**/{pattern}" if recursive else pattern | ||
| results: list[ConversionResult] = [] | ||
| batch = BatchConversionResult() | ||
|
|
||
| for input_path in sorted(input_dir.glob(glob_pattern)): | ||
| if input_path.is_dir(): | ||
| continue | ||
| if detect_format(str(input_path)) != input_format: | ||
| detected = detect_format(str(input_path)) | ||
| if detected != input_format: | ||
| batch.skipped.append( | ||
| { | ||
| "file": str(input_path), | ||
| "detected_format": detected or "unknown", | ||
| } | ||
| ) | ||
| continue | ||
|
|
||
| # Preserve relative path structure | ||
|
|
@@ -631,9 +659,9 @@ def convert_batch( | |
| output_format, | ||
| **writer_kwargs, | ||
| ) | ||
| results.append(result) | ||
| batch.results.append(result) | ||
|
|
||
| return results | ||
| return batch | ||
|
|
||
|
|
||
| def _format_to_extension(fmt: str) -> str: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| """Regression tests: zero-row Avro output artifact + CLI schema-file hardening.""" | ||
| from __future__ import annotations | ||
|
|
||
| import json | ||
|
|
||
| from click.testing import CliRunner | ||
|
|
||
| from datamorph.cli import cli | ||
| from datamorph.converters import convert | ||
|
|
||
|
|
||
| def _csv(tmp_path, rows="name,age\nalice,30\n"): | ||
| p = tmp_path / "in.csv" | ||
| p.write_text(rows, encoding="utf-8") | ||
| return str(p) | ||
|
|
||
|
|
||
| def test_zero_row_avro_output_file_exists_and_is_valid(tmp_path): | ||
| src = _csv(tmp_path, rows="name,age\n") # header only -> zero data rows | ||
| out = tmp_path / "out.avro" | ||
| result = convert(src, out) | ||
| assert not result.errors | ||
| assert result.rows_written == 0 | ||
| assert out.exists(), "empty conversion must still create the output file" | ||
| import fastavro | ||
|
|
||
| with open(out, "rb") as f: | ||
| rows = list(fastavro.reader(f)) | ||
| assert rows == [] | ||
|
|
||
|
|
||
| def test_validate_cmd_bad_json_schema_file_clean_exit(tmp_path): | ||
| data = _csv(tmp_path) | ||
| bad = tmp_path / "schema.json" | ||
| bad.write_text("{not valid json", encoding="utf-8") | ||
| r = CliRunner().invoke(cli, ["validate", data, "--schema", str(bad)]) | ||
| assert r.exit_code == 1 | ||
| assert "Could not load schema file" in r.output | ||
|
|
||
|
|
||
| def test_validate_cmd_wrong_shape_schema_file_clean_exit(tmp_path): | ||
| data = _csv(tmp_path) | ||
| bad = tmp_path / "schema.json" | ||
| bad.write_text(json.dumps({"name": "x"}), encoding="utf-8") | ||
| r = CliRunner().invoke(cli, ["validate", data, "--schema", str(bad)]) | ||
| assert r.exit_code == 1 | ||
| assert "non-empty JSON list" in r.output | ||
|
|
||
|
|
||
| def test_validate_cmd_good_schema_file_still_works(tmp_path): | ||
| data = _csv(tmp_path) | ||
| schema = [{"name": "name", "type": "string"}, {"name": "age", "type": "string"}] | ||
| good = tmp_path / "schema.json" | ||
| good.write_text(json.dumps(schema), encoding="utf-8") | ||
| r = CliRunner().invoke(cli, ["validate", data, "--schema", str(good)]) | ||
| assert r.exit_code == 0 | ||
| assert "VALID" in r.output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a nonempty file is inspected with
--sample 0or a negative value,FormatReader.infer_schemastill appends the first row before checking its stopping condition, while this new footer claims that at most zero (or a negative number of) rows were sampled. This both misreports the operation and silently infers a schema from only one row; constrain the Click option to positive integers or make inference honor zero consistently.Useful? React with 👍 / 👎.