kaxil commented on code in PR #70302:
URL: https://github.com/apache/airflow/pull/70302#discussion_r3703588226
##########
providers/common/ai/tests/unit/common/ai/utils/test_file_analysis.py:
##########
@@ -378,19 +382,29 @@ def
test_detect_file_format_rejects_unsupported_compression(self, tmp_path):
with pytest.raises(LLMFileAnalysisUnsupportedFormatError,
match="Compression"):
detect_file_format(ObjectStoragePath(str(path)))
- @pytest.mark.parametrize("filename", ["sample.parquet.gz",
"sample.avro.gz", "sample.png.gz"])
- def
test_detect_file_format_rejects_unsupported_gzip_format_combinations(self,
tmp_path, filename):
+ @pytest.mark.parametrize(
+ "filename", ["sample.parquet.gz", "sample.avro.bz2", "sample.png.xz",
"sample.pdf.gz"]
+ )
+ def
test_detect_file_format_rejects_unsupported_compression_format_combinations(self,
tmp_path, filename):
path = tmp_path / filename
path.write_bytes(b"content")
with pytest.raises(LLMFileAnalysisUnsupportedFormatError, match="not
supported for"):
Review Comment:
This `match` can't tell the two rejection branches apart: the codec-level
message ("Compression 'bzip2' is not supported for file analysis.") contains
the same substring, so the new `sample.avro.bz2` and `sample.png.xz` cases
would still pass if bzip2/xz support were reverted. Something like `match=r"not
supported for '\w+' file analysis"` pins them to the format-combination branch.
##########
providers/common/ai/src/airflow/providers/common/ai/utils/file_analysis.py:
##########
@@ -18,11 +18,13 @@
from __future__ import annotations
+import bz2
import csv
import gzip
import io
import json
import logging
+import lzma
Review Comment:
`_bz2` and `_lzma` are optional CPython extensions, so on an interpreter
built without them (the classic pyenv source build without liblzma-dev) this
import now takes down the whole module, and with it `LLMFileAnalysisOperator`
and `@task.llm_file_analysis`, even for the plain and gzip inputs that worked
before this PR. Since `detect_file_format` already keys off `_DECOMPRESSORS`
membership, registering bzip2/xz under `try`/`except ImportError` would degrade
those builds to the existing "Compression ... is not supported" error instead
(pandas guards these two imports the same way).
##########
providers/common/ai/docs/operators/llm_file_analysis.rst:
##########
@@ -155,9 +155,10 @@ Supported Formats
- Text-like: ``.log``, ``.json``, ``.csv``, ``.parquet``, ``.avro``
- Multimodal: ``.png``, ``.jpg``, ``.jpeg``, ``.pdf`` when ``multi_modal=True``
-- Gzip-compressed text inputs are supported for ``.log.gz``, ``.json.gz``, and
- ``.csv.gz``.
-- Gzip is not supported for ``.parquet``, ``.avro``, image, or PDF inputs.
+- ``gzip``, ``bzip2``, and ``xz`` compressed text inputs are supported for
+ ``.log``, ``.json``, and ``.csv`` (``.log.gz``, ``.csv.bz2``, ``.json.xz``,
...).
Review Comment:
The code also accepts compressed `.txt` here
(`_COMPRESSION_SUPPORTED_FORMATS` includes `txt`, and `test_detect_file_format`
covers `notes.txt.gz`), so `.txt` belongs in this list. The Text-like bullet
above is missing `.txt` too since #70431 didn't touch this page, may as well
fix both while you're here.
--
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]
