requests-cache icon indicating copy to clipboard operation
requests-cache copied to clipboard

SqLite: cannot set 'db_path'

Open njam opened this issue 1 year ago • 3 comments

The problem

I'm trying to set the db_path argument for the SqLite backend, but it's not accepted, instead it's using the cache_name as a filename as the path.

session = requests_cache.CachedSession('foo', backend='sqlite', db_path='/tmp/bar.sqlite')
print(f'Using cache location: {session.cache.db_path}')

Will print "Cache location: /foo.sqlite"

Expected behavior

Should print "Cache location: /tmp/bar.sqlite" and use it as the path.

Steps to reproduce the behavior

See above

Workarounds

Can create the backend explicitly:

self.session = CachedSession('foo', backend=SQLiteCache(db_path='/tmp/bar.sqlite'))

Investigation

The SqLite backend is expecting db_path to be an absolute path (code example). But for some reason the db_path argument is removed and replaced with cache_name in the init_backend() function.

Environment

  • requests-cache version: 1.2.1
  • Python version: 3.12
  • Platform: Arch

njam avatar Aug 07 '24 09:08 njam

Thank you for the details.

I have no hands-on experience with this library, but what you named as a "workaround" looks like a documented behavior in the Cache Files section. It means you should use backend=SQLiteCache(...) with the backed-specific options. Jordan Cook can correct me if I am wrong.

Anyway, I believe there is at least one problem with the library:

import requests
import requests_cache

session = requests_cache.CachedSession(non_existing_argument="foobar")

session = requests.Session(non_existing_argument="foobar")

where requests_cache.CachedSession silently accepts any unused/wrong arguments (this is a bad behavior)

and requests.Session raises

Traceback (most recent call last):
  File "/opt/pysetup/.try_requests.py", line 6, in <module>
    session = requests.Session(fake_argument="foobar")
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: Session.__init__() got an unexpected keyword argument 'non_existing_argument'

as it expected to be.

Of course, if db_path is expected, then the reported issue must be fixed, but if this is an unexpected argument, then we must raise an error.

alessio-locatelli avatar Aug 07 '24 09:08 alessio-locatelli

You can use either the first positional argument (cache_name):

CachedSession('/tmp/bar.sqlite', backend='sqlite')

Or db_path:

CachedSession(db_path='/tmp/bar.sqlite', backend='sqlite' )

But not both. cache_name has a different meaning for each backend. db_path is just an alias that makes it more explicit what it's used for in the SQLite backend. (I'd rather pick just one or the other, but have to keep both for backwards-compatibility).

Like @olk-m mentioned it's mostly documented here, but I'll try to make that less confusing, and raise an error when passing both args.

Unused/invalid backend args currently don't raise an error because they're passed to multiple classes via **kwargs, but I'll add a separate issue for that (#1006).

Thanks for the feedback!

JWCook avatar Aug 07 '24 15:08 JWCook

Ah got it now, thanks for your feedback!

Yeah I think an error message when both cache_name and db_path are passed would be useful, and would potentially safe those who don't read the docs thouroughly - like me - some time :O

njam avatar Aug 07 '24 19:08 njam

Continued in #1006

JWCook avatar Sep 05 '24 16:09 JWCook