Opened 3 years ago

Closed 3 years ago

#19100 closed Bug (invalid)

@cache_page argument parsing is misleading

Reported by: ksamuel Owned by: nobody
Component: Core (Cache system) Version: 1.4
Severity: Normal Keywords: cache, arguments
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Yesterday we moved:

@cache_page(max_age=60*30)

Which was working fine. To:

@cache_page(max_age=60*30, cache="alternative_cache")

Which triggered the following error:

The only keyword arguments are cache and key_prefix

Which is not true since @cache_page(max_age=60*30) worked fine before.

Of course, after fiddling a bit, we realized that:

@cache_page(60*30, cache="alternative_cache")

Worked. And we were a bit confused.

The source code of cache_page shows this:

def cache_page(*args, **kwargs):
    ...
    cache_alias = kwargs.pop('cache', None)
    key_prefix = kwargs.pop('key_prefix', None)
    assert not kwargs, "The only keyword arguments are cache and key_prefix"

I understand this is for legacy reasons, as I can see the deprecation warning below these lines:

    def warn():
        import warnings
        warnings.warn('The cache_page decorator must be called like: '
                      'cache_page(timeout, [cache=cache name], [key_prefix=key prefix]). '
                      'All other ways are deprecated.',
                      PendingDeprecationWarning,
                      stacklevel=3)

But then it should accept max_age as a keyword argument and raise the deprecation warning OR not accept it at all and have an explicit mandatory first positional argument being max_age. But not one behavior in one case, and another in the other case.

Change History (1)

comment:1 Changed 3 years ago by lukeplant

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to closed

I cannot reproduce this. @cache_page(max_age=60*30) throws an exception for me on 1.4 and master, which matches the documented warning and our intention. The docs are also clear on this.

Note: See TracTickets for help on using tickets.
Back to Top