Repository: drivendataorg/cloudpathlib
Pull request #426 (2024-04-09 16:17:27)

Fixes typo in Caching envvar (#424)
Live tests for #424

----------

* Fixes typo in Caching envvar

Envvar to set caching mode was previously CLOUPATHLIB_FILE_CACHE_MODE (missing D in CLOUDPATHLIB). This commit fixes this typo and introduces some code to help migrate projects to the new envvar. The current logic for migration warnings is:

- If the old envvar is set and not the new one, use the old one and issue a deprecation warning
- If the old and new envvars are set, use the new version and issue a warning if the two disagree

* Added deprecation version

* Fully reset envvars after testing


github-actions[bot]: <!-- NETLIFY DEPLOY COMMENT GENERATED BY ACTIONS_NETLIFY - APP ID SHA256: 9d0998be292ddace9530fffad95434f4e2279530e25cf278a25f07954faa426e -->
🚀 Deployed on https://deploy-preview-426--gallant-agnesi-5f7bb3.netlify.app
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/drivendataorg/cloudpathlib/pull/426?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 93.4%. Comparing base [(`d368501`)](https://app.codecov.io/gh/drivendataorg/cloudpathlib/commit/d3685011ba8dab516a13df509b388ee56eda2320?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg) to head [(`65e9ad2`)](https://app.codecov.io/gh/drivendataorg/cloudpathlib/pull/426?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg).

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff            @@
##           master    #426     +/-   ##
========================================
- Coverage    93.8%   93.4%   -0.4%     
========================================
  Files          23      23             
  Lines        1639    1646      +7     
========================================
+ Hits         1538    1539      +1     
- Misses        101     107      +6     
```


| [Files](https://app.codecov.io/gh/drivendataorg/cloudpathlib/pull/426?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg) | Coverage Δ | |
|---|---|---|
| [cloudpathlib/enums.py](https://app.codecov.io/gh/drivendataorg/cloudpathlib/pull/426?src=pr&el=tree&filepath=cloudpathlib%2Fenums.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg#diff-Y2xvdWRwYXRobGliL2VudW1zLnB5) | `100.0% <100.0%> (ø)` | |

... and [3 files with indirect coverage changes](https://app.codecov.io/gh/drivendataorg/cloudpathlib/pull/426/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg)

</details>

Patch:
diff --git a/HISTORY.md b/HISTORY.md
index 0b1ecdf..c56f815 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -2,6 +2,7 @@
 
 ## UNRELEASED
 
+- Fixes typo in `FileCacheMode` where values were being filled by envvar `CLOUPATHLIB_FILE_CACHE_MODE` instead of `CLOUDPATHLIB_FILE_CACHE_MODE`. (PR [#424](https://github.com/drivendataorg/cloudpathlib/pull/424)
 - Fix `CloudPath` cleanup via `CloudPath.__del__` when `Client` encounters an exception during initialization and does not create a `file_cache_mode` attribute. (Issue [#372](https://github.com/drivendataorg/cloudpathlib/issues/372), thanks to [@bryanwweber](https://github.com/bryanwweber)) 
 - Drop support for Python 3.7; pin minimal `boto3` version to Python 3.8+ versions. (PR [#407](https://github.com/drivendataorg/cloudpathlib/pull/407))
 - fix: use native `exists()` method in `GSClient`. (PR [#420](https://github.com/drivendataorg/cloudpathlib/pull/420))
diff --git a/cloudpathlib/enums.py b/cloudpathlib/enums.py
index 5f49ef1..408458d 100644
--- a/cloudpathlib/enums.py
+++ b/cloudpathlib/enums.py
@@ -1,4 +1,5 @@
 from enum import Enum
+import warnings
 import os
 from typing import Optional
 
@@ -16,7 +17,7 @@ class FileCacheMode(str, Enum):
         close_file (str): Cache for a `CloudPath` file is removed as soon as the file is closed. Note: you must
             use `CloudPath.open` whenever opening the file for this method to function.
 
-    Modes can be set by passing them to the Client or by setting the `CLOUPATHLIB_FILE_CACHE_MODE`
+    Modes can be set by passing them to the Client or by setting the `CLOUDPATHLIB_FILE_CACHE_MODE`
     environment variable.
 
     For more detail, see the [caching documentation page](../../caching).
@@ -29,14 +30,33 @@ class FileCacheMode(str, Enum):
 
     @classmethod
     def from_environment(cls) -> Optional["FileCacheMode"]:
-        """Parses the environment variable `CLOUPATHLIB_FILE_CACHE_MODE` into
+        """Parses the environment variable `CLOUDPATHLIB_FILE_CACHE_MODE` into
         an instance of this Enum.
 
         Returns:
             FileCacheMode enum value if the env var is defined, else None.
         """
-        env_string = os.environ.get("CLOUPATHLIB_FILE_CACHE_MODE", "").lower()
 
+        env_string = os.environ.get("CLOUDPATHLIB_FILE_CACHE_MODE", "").lower()
+        env_string_typo = os.environ.get("CLOUPATHLIB_FILE_CACHE_MODE", "").lower()
+
+        if env_string_typo:
+            warnings.warn(
+                "envvar CLOUPATHLIB_FILE_CACHE_MODE has been renamed to "
+                "CLOUDPATHLIB_FILE_CACHE_MODE. Reading from the old value "
+                "will become deprecated in version 0.20.0",
+                DeprecationWarning,
+            )
+
+        if env_string and env_string_typo and env_string != env_string_typo:
+            warnings.warn(
+                "CLOUDPATHLIB_FILE_CACHE_MODE and CLOUPATHLIB_FILE_CACHE_MODE "
+                "envvars set to different values. Disregarding old value and "
+                f"using CLOUDPATHLIB_FILE_CACHE_MODE = {env_string}",
+                RuntimeWarning,
+            )
+
+        env_string = env_string or env_string_typo
         if not env_string:
             return None
         else:
diff --git a/docs/docs/caching.ipynb b/docs/docs/caching.ipynb
index 16a2c19..c6d627a 100644
--- a/docs/docs/caching.ipynb
+++ b/docs/docs/caching.ipynb
@@ -460,9 +460,9 @@
     "\n",
     "### Setting the cache clearing method\n",
     "\n",
-    "You can set the cache clearing method either through the environment variable `CLOUPATHLIB_FILE_CACHE_MODE` or by passing the mode to the `*Client` when you instantiate it. See below for an example.\n",
+    "You can set the cache clearing method either through the environment variable `CLOUDPATHLIB_FILE_CACHE_MODE` or by passing the mode to the `*Client` when you instantiate it. See below for an example.\n",
     "\n",
-    "You can set `CLOUPATHLIB_FILE_CACHE_MODE` to any of the supported values, which are printed below."
+    "You can set `CLOUDPATHLIB_FILE_CACHE_MODE` to any of the supported values, which are printed below."
    ]
   },
   {
@@ -727,7 +727,7 @@
      "traceback": [
       "\u001b[0;31m---------------------------------------------------------------------------\u001b[0m",
       "\u001b[0;31mInvalidConfigurationException\u001b[0m             Traceback (most recent call last)",
-      "Cell \u001b[0;32mIn[18], line 6\u001b[0m\n\u001b[1;32m      3\u001b[0m \u001b[38;5;66;03m# set the mode here so that it will be used when we instantiate the client\u001b[39;00m\n\u001b[1;32m      4\u001b[0m os\u001b[38;5;241m.\u001b[39menviron[\u001b[38;5;124m\"\u001b[39m\u001b[38;5;124mCLOUPATHLIB_FILE_CACHE_MODE\u001b[39m\u001b[38;5;124m\"\u001b[39m] \u001b[38;5;241m=\u001b[39m \u001b[38;5;124m\"\u001b[39m\u001b[38;5;124mpersistent\u001b[39m\u001b[38;5;124m\"\u001b[39m\n\u001b[0;32m----> 6\u001b[0m tmp_dir_client \u001b[38;5;241m=\u001b[39m \u001b[43mS3Client\u001b[49m\u001b[43m(\u001b[49m\u001b[43m)\u001b[49m\n",
+      "Cell \u001b[0;32mIn[18], line 6\u001b[0m\n\u001b[1;32m      3\u001b[0m \u001b[38;5;66;03m# set the mode here so that it will be used when we instantiate the client\u001b[39;00m\n\u001b[1;32m      4\u001b[0m os\u001b[38;5;241m.\u001b[39menviron[\u001b[38;5;124m\"\u001b[39m\u001b[38;5;124mCLOUDPATHLIB_FILE_CACHE_MODE\u001b[39m\u001b[38;5;124m\"\u001b[39m] \u001b[38;5;241m=\u001b[39m \u001b[38;5;124m\"\u001b[39m\u001b[38;5;124mpersistent\u001b[39m\u001b[38;5;124m\"\u001b[39m\n\u001b[0;32m----> 6\u001b[0m tmp_dir_client \u001b[38;5;241m=\u001b[39m \u001b[43mS3Client\u001b[49m\u001b[43m(\u001b[49m\u001b[43m)\u001b[49m\n",
       "File \u001b[0;32m~/code/cloudpathlib/cloudpathlib/s3/s3client.py:129\u001b[0m, in \u001b[0;36mS3Client.__init__\u001b[0;34m(self, aws_access_key_id, aws_secret_access_key, aws_session_token, no_sign_request, botocore_session, profile_name, boto3_session, file_cache_mode, local_cache_dir, endpoint_url, boto3_transfer_config, content_type_method, extra_args)\u001b[0m\n\u001b[1;32m    121\u001b[0m \u001b[38;5;66;03m# listing ops (list_objects_v2, filter, delete) only accept these extras:\u001b[39;00m\n\u001b[1;32m    122\u001b[0m \u001b[38;5;66;03m# https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html\u001b[39;00m\n\u001b[1;32m    123\u001b[0m \u001b[38;5;28mself\u001b[39m\u001b[38;5;241m.\u001b[39mboto3_list_extra_args \u001b[38;5;241m=\u001b[39m {\n\u001b[1;32m    124\u001b[0m     k: \u001b[38;5;28mself\u001b[39m\u001b[38;5;241m.\u001b[39m_extra_args[k]\n\u001b[1;32m    125\u001b[0m     \u001b[38;5;28;01mfor\u001b[39;00m k \u001b[38;5;129;01min\u001b[39;00m [\u001b[38;5;124m\"\u001b[39m\u001b[38;5;124mRequestPayer\u001b[39m\u001b[38;5;124m\"\u001b[39m, \u001b[38;5;124m\"\u001b[39m\u001b[38;5;124mExpectedBucketOwner\u001b[39m\u001b[38;5;124m\"\u001b[39m]\n\u001b[1;32m    126\u001b[0m     \u001b[38;5;28;01mif\u001b[39;00m k \u001b[38;5;129;01min\u001b[39;00m \u001b[38;5;28mself\u001b[39m\u001b[38;5;241m.\u001b[39m_extra_args\n\u001b[1;32m    127\u001b[0m }\n\u001b[0;32m--> 129\u001b[0m \u001b[38;5;28;43msuper\u001b[39;49m\u001b[43m(\u001b[49m\u001b[43m)\u001b[49m\u001b[38;5;241;43m.\u001b[39;49m\u001b[38;5;21;43m__init__\u001b[39;49m\u001b[43m(\u001b[49m\n\u001b[1;32m    130\u001b[0m \u001b[43m    \u001b[49m\u001b[43mlocal_cache_dir\u001b[49m\u001b[38;5;241;43m=\u001b[39;49m\u001b[43mlocal_cache_dir\u001b[49m\u001b[43m,\u001b[49m\n\u001b[1;32m    131\u001b[0m \u001b[43m    \u001b[49m\u001b[43mcontent_type_method\u001b[49m\u001b[38;5;241;43m=\u001b[39;49m\u001b[43mcontent_type_method\u001b[49m\u001b[43m,\u001b[49m\n\u001b[1;32m    132\u001b[0m \u001b[43m    \u001b[49m\u001b[43mfile_cache_mode\u001b[49m\u001b[38;5;241;43m=\u001b[39;49m\u001b[43mfile_cache_mode\u001b[49m\u001b[43m,\u001b[49m\n\u001b[1;32m    133\u001b[0m \u001b[43m\u001b[49m\u001b[43m)\u001b[49m\n",
       "File \u001b[0;32m~/code/cloudpathlib/cloudpathlib/client.py:57\u001b[0m, in \u001b[0;36mClient.__init__\u001b[0;34m(self, file_cache_mode, local_cache_dir, content_type_method)\u001b[0m\n\u001b[1;32m     54\u001b[0m     file_cache_mode \u001b[38;5;241m=\u001b[39m FileCacheMode\u001b[38;5;241m.\u001b[39mpersistent\n\u001b[1;32m     56\u001b[0m \u001b[38;5;28;01mif\u001b[39;00m file_cache_mode \u001b[38;5;241m==\u001b[39m FileCacheMode\u001b[38;5;241m.\u001b[39mpersistent \u001b[38;5;129;01mand\u001b[39;00m local_cache_dir \u001b[38;5;129;01mis\u001b[39;00m \u001b[38;5;28;01mNone\u001b[39;00m:\n\u001b[0;32m---> 57\u001b[0m     \u001b[38;5;28;01mraise\u001b[39;00m InvalidConfigurationException(\n\u001b[1;32m     58\u001b[0m         \u001b[38;5;124mf\u001b[39m\u001b[38;5;124m\"\u001b[39m\u001b[38;5;124mIf you use the \u001b[39m\u001b[38;5;124m'\u001b[39m\u001b[38;5;132;01m{\u001b[39;00mFileCacheMode\u001b[38;5;241m.\u001b[39mpersistent\u001b[38;5;132;01m}\u001b[39;00m\u001b[38;5;124m'\u001b[39m\u001b[38;5;124m cache mode, you must pass a `local_cache_dir` when you instantiate the client.\u001b[39m\u001b[38;5;124m\"\u001b[39m\n\u001b[1;32m     59\u001b[0m     )\n\u001b[1;32m     61\u001b[0m \u001b[38;5;66;03m# if no explicit local dir, setup caching in temporary dir\u001b[39;00m\n\u001b[1;32m     62\u001b[0m \u001b[38;5;28;01mif\u001b[39;00m local_cache_dir \u001b[38;5;129;01mis\u001b[39;00m \u001b[38;5;28;01mNone\u001b[39;00m:\n",
       "\u001b[0;31mInvalidConfigurationException\u001b[0m: If you use the 'FileCacheMode.persistent' cache mode, you must pass a `local_cache_dir` when you instantiate the client."
@@ -738,7 +738,7 @@
     "import os\n",
     "\n",
     "# set the mode here so that it will be used when we instantiate the client\n",
-    "os.environ[\"CLOUPATHLIB_FILE_CACHE_MODE\"] = \"persistent\"\n",
+    "os.environ[\"CLOUDPATHLIB_FILE_CACHE_MODE\"] = \"persistent\"\n",
     "\n",
     "tmp_dir_client = S3Client()"
    ]
diff --git a/docs/docs/script/caching.py b/docs/docs/script/caching.py
index 9608567..d8bb5b6 100644
--- a/docs/docs/script/caching.py
+++ b/docs/docs/script/caching.py
@@ -182,9 +182,9 @@ get_ipython().system('rm -rf data')
 # 
 # ### Setting the cache clearing method
 # 
-# You can set the cache clearing method either through the environment variable `CLOUPATHLIB_FILE_CACHE_MODE` or by passing the mode to the `*Client` when you instantiate it. See below for an example.
+# You can set the cache clearing method either through the environment variable `CLOUDPATHLIB_FILE_CACHE_MODE` or by passing the mode to the `*Client` when you instantiate it. See below for an example.
 # 
-# You can set `CLOUPATHLIB_FILE_CACHE_MODE` to any of the supported values, which are printed below.
+# You can set `CLOUDPATHLIB_FILE_CACHE_MODE` to any of the supported values, which are printed below.
 
 from cloudpathlib.enums import FileCacheMode
 
@@ -331,7 +331,7 @@ shutil.rmtree(client_cache_dir)
 import os
 
 # set the mode here so that it will be used when we instantiate the client
-os.environ["CLOUPATHLIB_FILE_CACHE_MODE"] = "persistent"
+os.environ["CLOUDPATHLIB_FILE_CACHE_MODE"] = "persistent"
 
 tmp_dir_client = S3Client()
 


Test patch:
diff --git a/tests/test_caching.py b/tests/test_caching.py
index d39f09e..af1aebc 100644
--- a/tests/test_caching.py
+++ b/tests/test_caching.py
@@ -270,11 +270,32 @@ def test_string_instantiation(rig: CloudProviderTestRig, tmpdir):
         assert client.file_cache_mode == v
 
 
-def test_environment_variable_instantiation(rig: CloudProviderTestRig, tmpdir):
+def test_environment_variable_contentious_instantiation(rig: CloudProviderTestRig, tmpdir):
+    # environment instantiation
+    original_typo_env_setting = os.environ.get("CLOUPATHLIB_FILE_CACHE_MODE", "")
+    original_env_setting = os.environ.get("CLOUDPATHLIB_FILE_CACHE_MODE", "")
+
+    v_old = FileCacheMode.persistent
+    try:
+        for v in FileCacheMode:
+            os.environ["CLOUPATHLIB_FILE_CACHE_MODE"] = v_old.value
+            os.environ["CLOUDPATHLIB_FILE_CACHE_MODE"] = v.value
+            local = tmpdir if v == FileCacheMode.persistent else None
+            client = rig.client_class(local_cache_dir=local, **rig.required_client_kwargs)
+            assert client.file_cache_mode == v
+
+    finally:
+        os.environ["CLOUPATHLIB_FILE_CACHE_MODE"] = original_typo_env_setting
+        os.environ["CLOUDPATHLIB_FILE_CACHE_MODE"] = original_env_setting
+
+
+def test_environment_variable_old_instantiation(rig: CloudProviderTestRig, tmpdir):
     # environment instantiation
-    original_env_setting = os.environ.get("CLOUPATHLIB_FILE_CACHE_MODE", "")
+    original_typo_env_setting = os.environ.get("CLOUPATHLIB_FILE_CACHE_MODE", "")
+    original_env_setting = os.environ.get("CLOUDPATHLIB_FILE_CACHE_MODE", "")
 
     try:
+        os.environ["CLOUDPATHLIB_FILE_CACHE_MODE"] = ""
         for v in FileCacheMode:
             os.environ["CLOUPATHLIB_FILE_CACHE_MODE"] = v.value
             local = tmpdir if v == FileCacheMode.persistent else None
@@ -282,7 +303,23 @@ def test_environment_variable_instantiation(rig: CloudProviderTestRig, tmpdir):
             assert client.file_cache_mode == v
 
     finally:
-        os.environ["CLOUPATHLIB_FILE_CACHE_MODE"] = original_env_setting
+        os.environ["CLOUPATHLIB_FILE_CACHE_MODE"] = original_typo_env_setting
+        os.environ["CLOUDPATHLIB_FILE_CACHE_MODE"] = original_env_setting
+
+
+def test_environment_variable_instantiation(rig: CloudProviderTestRig, tmpdir):
+    # environment instantiation
+    original_env_setting = os.environ.get("CLOUDPATHLIB_FILE_CACHE_MODE", "")
+
+    try:
+        for v in FileCacheMode:
+            os.environ["CLOUDPATHLIB_FILE_CACHE_MODE"] = v.value
+            local = tmpdir if v == FileCacheMode.persistent else None
+            client = rig.client_class(local_cache_dir=local, **rig.required_client_kwargs)
+            assert client.file_cache_mode == v
+
+    finally:
+        os.environ["CLOUDPATHLIB_FILE_CACHE_MODE"] = original_env_setting
 
 
 def test_environment_variable_local_cache_dir(rig: CloudProviderTestRig, tmpdir):