potiuk commented on code in PR #37166:
URL: https://github.com/apache/airflow/pull/37166#discussion_r1535741587


##########
tests/providers/common/io/xcom/test_backend.py:
##########
@@ -86,28 +87,19 @@ def task_instance(task_instance_factory):
 
 
 class TestXcomObjectStoreBackend:
-    path = "file:/tmp/xcom"
-
-    def setup_method(self):
-        try:
-            conf.add_section("common.io")
-        except DuplicateSectionError:
-            pass
-        conf.set("core", "xcom_backend", 
"airflow.providers.common.io.xcom.backend.XComObjectStoreBackend")
-        conf.set("common.io", "xcom_objectstore_path", self.path)
-        conf.set("common.io", "xcom_objectstore_threshold", "50")
-        settings.configure_vars()
-
-    def teardown_method(self):
-        conf.remove_option("core", "xcom_backend")
-        conf.remove_option("common.io", "xcom_objectstore_path")
-        conf.remove_option("common.io", "xcom_objectstore_threshold")
-        settings.configure_vars()
-        p = ObjectStoragePath(self.path)
-        if p.exists():
-            p.rmdir(recursive=True)
-
-    @pytest.mark.db_test
+    @pytest.fixture(autouse=True)
+    def setup_test_cases(self, tmp_path):
+        xcom_path = tmp_path / "xcom"
+        xcom_path.mkdir()
+        self.path = f"file:{xcom_path}"

Review Comment:
   IMHO
   
   > 1) When [common.io] xcom_objectstore_path set to file:/foo/bar XComBackend 
use absolute path /foo/bar
   > 2) When [common.io] xcom_objectstore_path set to file://foo/bar 
XComBackend use relative to the current directory path foo/bar
   > 3) When [common.io] xcom_objectstore_path set to file:///foo/bar 
XComBackend failed with an error ValueError: Invalid key
   
   1) 2) are ok. 1) is perfectly fine according to RFC3986 and explained nicely 
here https://en.wikipedia.org/wiki/File_URI_scheme 2) is not correct (`://` 
should be followed by eiter '/' or literal localhost,) but many implementation 
accept it as a shortcut
   
   3) invalid key should be accepted - it's fully valid according to RFC and 
many implementations and should be `/foo/bar` absolute).
   
   But maybe it's been fixed in the last iteration already ?
   



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