kaxil commented on code in PR #75188:
URL: https://github.com/apache/airflow/pull/75188#discussion_r4323244462


##########
providers/common/ai/tests/unit/common/ai/sandbox/test_modal.py:
##########
@@ -144,7 +144,10 @@ def 
test_create_accepts_every_argument_the_backend_passes(self, real_modal):
     def test_the_calls_behind_the_tools_still_exist(self, real_modal):
         import inspect
 
-        from modal.sandbox import _SandboxFilesystem
+        try:
+            from modal.sandbox._filesystem import _SandboxFilesystem
+        except ImportError:  # modal < 1.6 keeps it in a flat module

Review Comment:
   Small correction on the comment: 1.6.0 and 1.6.1 still have the flat 
`modal.sandbox` (the class is defined in `modal.sandbox_fs` there and 
re-exported), the move landed in 1.6.2. I checked 1.5.2, 1.5.5, 1.6.0, 1.6.1 
and 1.6.2.
   
   You could also drop the private path altogether. Production reaches this 
class through `handle.filesystem`, and `Sandbox.filesystem`'s getter is 
annotated with it on all five of those versions, so 
`inspect.signature(real_modal.Sandbox.filesystem.fget).return_annotation` 
returns the same class with no try/except. That way the next internal move in 
Modal doesn't trip a test that's meant to catch API changes.



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