shahar1 commented on code in PR #70628:
URL: https://github.com/apache/airflow/pull/70628#discussion_r3680547383


##########
providers/common/ai/src/airflow/providers/common/ai/operators/document_loader.py:
##########
@@ -148,13 +152,15 @@ def __init__(
         self.json_text_field = json_text_field
 
     def execute(self, context: Context) -> list[dict[str, Any]]:
-        # source_path/file_type are template fields; validate after rendering, 
not in __init__.
-        if self.source_path is not None and self.source_bytes is not None:
-            raise ValueError("Provide exactly one of 'source_path' or 
'source_bytes', not both.")
-        if self.source_path is None and self.source_bytes is None:
-            raise ValueError("Provide exactly one of 'source_path' or 
'source_bytes'.")
+        # file_type and source_path can each be *supplied* (as non-None 
argument) yet still
+        # render to None. These aren't provision checks (that already happened 
in __init__); 
+        # they guard the rendered value itself, since 
_parse_bytes/_resolve_files need a real 
+        # value to work with. Checking this in __init__ would validate the 
unrendered template 
+        # string instead of the value actually used here.
         if self.source_bytes is not None and self.file_type is None:
             raise ValueError("'file_type' is required when using 
'source_bytes' (e.g. '.pdf').")
+        if self.source_bytes is None and self.source_path is None:
+            raise ValueError("Provide exactly one of 'source_path' or 
'source_bytes'.")

Review Comment:
   Could you please add a unit test for this case?



-- 
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]

Reply via email to