Repository: pyinat/pyinaturalist

Issue:
get_projects_by_id doesn't use the session's limiter db path
### The problem

When using a session with a custom session with `bucket_class=FileLockSQLiteBucket` and `ratelimit_path` set to a filename in the user's home directory, that path is ignored when `get_projects_by_id()` is called with `force_refresh=True` (the default). Instead, it uses the default path `/tmp/pyrate_limiter.sqlite`.

This effectively prevents pyinaturalist code run by two different users on the same system from performing rate limited requests, as the second user's code can't write to the mode 0644 `/tmp/pyrate_limiter.sqlite` file owned by the first user.

### Expected behavior

I expected all limiters created by pyinaturalist to use the same `ratelimit_path` that I passed when I created my custom session instance, thereby avoiding conflicts like this when more than one user runs pyinaturalist code on the same system.

### Steps to reproduce the behavior

The bug is triggered in `dronefly` bot when running pyinaturalist code from two different accounts on the same system:
- `user1` calls `client.projects.delete_users()`, causing the DB to be created in `/tmp/pyrate_limiter.sqlite` instead of the client.session's `db_path` (should be `~/.local/share/dronefly-core/ratelimit.db` to avoid this problem)
- `user2`
  - running a different `dronefly` instance, makes the same call and it will conflict because the DB belongs to another user and therefore is opened read-only
  OR
  - running any pyinat code as `user2` using any endpoint but *without* a custom session while the `/tmp/pyrate_limiter.sqlite` file owned by `user1` exists will fail for the same reason; that should be fairly trivial to reproduce

### Workarounds

No workarounds that I'd be happy with exist.

### Environment
 - OS & version: Debian 12
 - Python version: 3.11
 - Pyinaturalist version or branch: `0.20.1` and `main`

Discussion:
@JWCook: I see, it looks like that was introduced in #363.

Trying to retrace my steps from 3 years ago, it looks like the problem was:
* We sometimes got stale project responses due to CDN caches in between the client and iNat servers
* These caches ignore `Cache-Control` headers to invalidate the cache, if the cached response was <2 minutes old
* I handled this the same way iNat does, but adding an extra (unused) request param to bust the cache when needed
* To determine when that's needed (every ~2 minutes), I used a separate limiter object

Now I just need to figure out a good way to make that function use the `ratelimit_path` configured on the session instead.

Solution patch:
diff --git a/HISTORY.md b/HISTORY.md
index 14ac43e..b8be73a 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -1,5 +1,8 @@
 # History
 
+## Unreleased
+* `ClientSession` arguments `ratelimit_path` and `lock_path` will now apply to the separate rate-limiter used for `/projects/{id}`
+
 ## 0.20.1 (2025-01-18)
 * Fix `AttributeError` when initializing `Observation.taxon` with a `Taxon` object
 
diff --git a/docs/user_guide/advanced.md b/docs/user_guide/advanced.md
index e6397cd..549c992 100644
--- a/docs/user_guide/advanced.md
+++ b/docs/user_guide/advanced.md
@@ -90,6 +90,11 @@ Float values also work, for example to slow it down to less than 1 request per s
 >>> session = ClientSession(per_second=0.5)
 ```
 
+Some persistent rate-limiting state is stored on-disk. To change the default path for this:
+```python
+>>> session = ClientSession(ratelimit_path='/tmp/ratelimit.db')
+```
+
 ### Distributed Application Rate Limiting
 The default rate-limiting backend is thread-safe, and persistent across application restarts. If
 you have a larger application running from multiple processes, you will need an additional locking
@@ -106,6 +111,11 @@ This requires installing one additional dependency, [py-filelock](https://github
 pip install filelock
 ```
 
+To change the default path used for the file lock:
+```python
+>>> session = ClientSession(bucket_class=FileLockSQLiteBucket, lock_path='/tmp/ratelimit.lock')
+```
+
 ## Logging
 You can configure logging for pyinaturalist using the standard Python `logging` module, for example
 with {py:func}`logging.basicConfig`:
diff --git a/pyinaturalist/constants.py b/pyinaturalist/constants.py
index 31ba576..0f9646a 100644
--- a/pyinaturalist/constants.py
+++ b/pyinaturalist/constants.py
@@ -6,7 +6,6 @@ from typing import IO, TYPE_CHECKING, Any, BinaryIO, Dict, Iterable, List, Optio
 
 from dateutil.relativedelta import relativedelta
 from platformdirs import user_data_dir
-from pyrate_limiter.sqlite_bucket import LOCK_PATH as DEFAULT_LOCK_PATH
 
 # iNaturalist URLs
 API_V0 = 'https://www.inaturalist.org'
@@ -66,6 +65,7 @@ CACHE_EXPIRATION = {
 }
 CACHE_FILE = join(DATA_DIR, 'api_requests.db')
 RATELIMIT_FILE = join(DATA_DIR, 'api_ratelimit.db')
+DEFAULT_LOCK_PATH = join(DATA_DIR, 'api_ratelimit.lock')
 
 # Response formats supported by v0 GET /observations endpoint
 OBSERVATION_FORMATS = ['atom', 'csv', 'dwc', 'json', 'kml', 'widget']
diff --git a/pyinaturalist/session.py b/pyinaturalist/session.py
index 5c711e0..b79be11 100644
--- a/pyinaturalist/session.py
+++ b/pyinaturalist/session.py
@@ -58,13 +58,6 @@ from pyinaturalist.request_params import (
     preprocess_request_params,
 )
 
-# Rate limiter specific to forced refresh request
-REFRESH_LIMITER = Limiter(
-    RequestRate(1, 122),
-    bucket_class=SQLiteBucket,
-    bucket_kwargs={'path': RATELIMIT_FILE},
-)
-
 logger = getLogger('pyinaturalist')
 thread_local = threading.local()
 
@@ -91,6 +84,8 @@ class ClientSession(CacheMixin, LimiterMixin, Session):
         burst: int = REQUEST_BURST_RATE,
         bucket_class: Type[AbstractBucket] = SQLiteBucket,
         backoff_factor: float = RETRY_BACKOFF,
+        ratelimit_path: Optional[str] = RATELIMIT_FILE,
+        lock_path: Optional[str] = DEFAULT_LOCK_PATH,
         max_retries: int = REQUEST_RETRIES,
         timeout: int = REQUEST_TIMEOUT,
         user_agent: Optional[str] = None,
@@ -110,8 +105,12 @@ class ClientSession(CacheMixin, LimiterMixin, Session):
             per_minute: Max requests per minute
             per_day: Max requests per day
             burst: Max number of consecutive requests allowed before applying per-second rate-limiting
-            bucket_class: Rate-limiting backend to use. Defaults to a persistent SQLite database.
+            bucket_class: Rate-limiting backend to use; defaults to a persistent SQLite database.
             backoff_factor: Factor for increasing delays between retries
+            ratelimit_path: Path to SQLite database for rate-limiting;
+                defaults to the system default cache directory
+            lock_path: Path to file lock for multiprocess rate-limit database;
+                 defaults to the system default cache directory
             max_retries: Maximum number of times to retry a failed request
             timeout: Maximum number of seconds to wait for a response from the server
             user_agent: Additional User-Agent info to pass to API requests
@@ -126,9 +125,9 @@ class ClientSession(CacheMixin, LimiterMixin, Session):
 
         # Extra args to pass to rate limiter backend
         bucket_kwargs = kwargs.pop('bucket_kwargs', {})
-        if ratelimit_path := kwargs.pop('ratelimit_path', None):
+        if ratelimit_path:
             bucket_kwargs['path'] = ratelimit_path
-        if lock_path := kwargs.pop('lock_path', None):
+        if lock_path and bucket_class is FileLockSQLiteBucket:
             bucket_kwargs['lock_path'] = lock_path
 
         super().__init__(  # type: ignore  # false positive
@@ -152,6 +151,13 @@ class ClientSession(CacheMixin, LimiterMixin, Session):
             **kwargs,
         )
 
+        # Separate rate limiter specific to forced refresh requests
+        self.refresh_limiter = Limiter(
+            RequestRate(1, 122),
+            bucket_class=bucket_class,
+            bucket_kwargs=bucket_kwargs,
+        )
+
         # Retry settings
         self.retries = Retry(
             total=max_retries, backoff_factor=backoff_factor, status_forcelist=RETRY_STATUSES
@@ -332,6 +338,28 @@ class ClientSession(CacheMixin, LimiterMixin, Session):
         logger.debug(format_response(response))
         return response
 
+    def get_refresh_params(self, endpoint) -> Dict:
+        """In some cases, we need to be sure we have the most recent version of a resource, for example
+        when updating projects. Normally we would handle this with cache headers, but the CDN cache does
+        not respect these if the cached response is less than ~2 minutes old.
+
+        The iNat webapp handles this by adding an extra `v` request parameter, which results in a
+        different cache key. This function does the same by tracking a separate rate limit per value,
+        and finding the lowest value that is certain to result in a fresh response.
+        """
+        v = 0
+        while True:
+            try:
+                bucket = f'{endpoint}?v={v}'
+                self.refresh_limiter.try_acquire(bucket)
+                break
+            except BucketFullException as e:
+                seconds = int(e.meta_info['remaining_time'])
+                logger.debug(f'{bucket} cannot be refreshed again for {seconds} seconds')
+                v += 1
+
+        return {'refresh': True, 'v': v} if v > 0 else {'refresh': True}
+
     def _validate_json(
         self,
         request: PreparedRequest,
@@ -437,29 +465,6 @@ def get_local_session(**kwargs) -> ClientSession:
     return thread_local.session
 
 
-def get_refresh_params(endpoint) -> Dict:
-    """In some cases, we need to be sure we have the most recent version of a resource, for example
-    when updating projects. Normally we would handle this with cache headers, but the CDN cache does
-    not respect these if the cached response is less than ~2 minutes old.
-
-    The iNat webapp handles this by adding an extra `v` request parameter, which results in a
-    different cache key. This function does the same by tracking a separate rate limit per value,
-    and finding the lowest value that is certain to result in a fresh response.
-    """
-    v = 0
-    while True:
-        try:
-            bucket = f'{endpoint}?v={v}'
-            REFRESH_LIMITER.try_acquire(bucket)
-            break
-        except BucketFullException as e:
-            seconds = int(e.meta_info['remaining_time'])
-            logger.debug(f'{bucket} cannot be refreshed again for {seconds} seconds')
-            v += 1
-
-    return {'refresh': True, 'v': v} if v > 0 else {'refresh': True}
-
-
 class MockResponse(CachedResponse):
     """A mock response to return in dry-run mode.
     This behaves the same as a cached response, but with the following additions:
diff --git a/pyinaturalist/v1/projects.py b/pyinaturalist/v1/projects.py
index 0d25254..ed42209 100644
--- a/pyinaturalist/v1/projects.py
+++ b/pyinaturalist/v1/projects.py
@@ -14,7 +14,7 @@ from pyinaturalist.docs import document_request_params
 from pyinaturalist.docs import templates as docs
 from pyinaturalist.paginator import paginate_all
 from pyinaturalist.request_params import split_common_params, validate_multiple_choice_param
-from pyinaturalist.session import delete, get, get_refresh_params, post, put
+from pyinaturalist.session import ClientSession, delete, get, get_local_session, post, put
 
 logger = getLogger(__name__)
 
@@ -70,6 +70,7 @@ def get_projects_by_id(
     project_id: MultiIntOrStr,
     rule_details: Optional[bool] = None,
     force_refresh: bool = False,
+    session: Optional[ClientSession] = None,
     **params,
 ) -> JsonResponse:
     """Get one or more projects by ID
@@ -100,12 +101,16 @@ def get_projects_by_id(
     Returns:
         Response dict containing project records
     """
+    # If needed, invalidate CDN cache to ensure a fresh response
+    session = session or get_local_session()
     if force_refresh:
-        params.update(get_refresh_params(f'/projects/{project_id}'))
-    response = get(
+        params.update(session.get_refresh_params(f'/projects/{project_id}'))
+
+    params['rule_details'] = rule_details
+    response = session.request(
+        'GET',
         f'{API_V1}/projects',
-        rule_details=rule_details,
-        ids=project_id,
+        ids=project_id,  # type: ignore[arg-type]
         allow_str_ids=True,
         **params,
     )


Test patch:
diff --git a/test/test_session.py b/test/test_session.py
index 1f717dd..c47bd24 100644
--- a/test/test_session.py
+++ b/test/test_session.py
@@ -18,7 +18,6 @@ from pyinaturalist.session import (
     delete,
     get,
     get_local_session,
-    get_refresh_params,
     post,
     put,
 )
@@ -253,11 +252,12 @@ def test_filelock(mock_send, tmp_path):
         assert bucket._lock.lock_file == lock_path
 
 
-@patch('pyinaturalist.session.REFRESH_LIMITER', Limiter(RequestRate(1, 2)))
 def test_get_refresh_params():
-    assert get_refresh_params('test') == {'refresh': True}
-    assert get_refresh_params('test2') == {'refresh': True}
-    assert get_refresh_params('test') == {'refresh': True, 'v': 1}
-    assert get_refresh_params('test') == {'refresh': True, 'v': 2}
+    session = ClientSession()
+    session.refresh_limiter = Limiter(RequestRate(1, 2))
+    assert session.get_refresh_params('test') == {'refresh': True}
+    assert session.get_refresh_params('test2') == {'refresh': True}
+    assert session.get_refresh_params('test') == {'refresh': True, 'v': 1}
+    assert session.get_refresh_params('test') == {'refresh': True, 'v': 2}
     sleep(2)
-    assert get_refresh_params('test') == {'refresh': True}
+    assert session.get_refresh_params('test') == {'refresh': True}