Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python module cache exception doesn't get formatted right #134

Closed
akx opened this issue Jan 26, 2013 · 6 comments
Closed

Python module cache exception doesn't get formatted right #134

akx opened this issue Jan 26, 2013 · 6 comments
Labels

Comments

@akx
Copy link
Contributor

akx commented Jan 26, 2013

While testing some functionality of the cache on IRC, I noticed that the exception for "cache key too large" isn't very helpful:

Traceback (most recent call last):
  File "./uw_cachetest.py", line 14, in application
    uwsgi.cache_update(key, data_in)
ValueError: uWSGI cache items size must be < %llu, requested %llu bytes

This is on 1.4.3.

@prymitive
Copy link
Contributor

Test case:

cachetest.py

import uwsgi

def application(environ, start_response):
    msg = ''.join([chr(97 + i % 25) for i in xrange(0, 8192)])
    uwsgi.cache_set(msg, msg)
    start_response('200 OK', [('Content-Type', 'text/html')])
    return 'OK'

command line

>$ uwsgi --logdate -M --http :8080 --wsgi-file cachetest.py --cache 1024 --cache-blocksize 1024
>$ curl -I http://127.0.0.1:8080/

Changing printed values from unsigned long long to just unsigned long will make it print them properly.
%llu is only avaiable since python 2.7 and it requires HAVE_LONG_LONG defined. I think that unsigned long is big enough for block size, but since it's stored as uint64_t it may rise issues if we do so.

Related docs:

Note The “%lld” and “%llu” format specifiers are only available when HAVE_LONG_LONG is defined.
Changed in version 2.7: Support for “%lld” and “%llu” added.

from python docs.

diff --git a/plugins/python/uwsgi_pymodule.c b/plugins/python/uwsgi_pymodule.c
index 634b8d4..aef4e00 100644
--- a/plugins/python/uwsgi_pymodule.c
+++ b/plugins/python/uwsgi_pymodule.c
@@ -3553,7 +3553,7 @@ PyObject *py_uwsgi_cache_set(PyObject * self, PyObject * args) {
        }

        if ((uint64_t)vallen > uwsgi.caches->blocksize) {
-               return PyErr_Format(PyExc_ValueError, "uWSGI cache items size must be < %llu, requested %llu bytes", (unsigned long long)uwsgi.caches->blocksize, (unsigned long long) vallen);
+               return PyErr_Format(PyExc_ValueError, "uWSGI cache items size must be < %lu, requested %lu bytes", (unsigned long)uwsgi.caches->blocksize, (unsigned long) vallen);
        }

        if (remote && strlen(remote) > 0) {
@@ -3594,7 +3594,7 @@ PyObject *py_uwsgi_cache_update(PyObject * self, PyObject * args) {
         }

         if ((uint64_t)vallen > uwsgi.caches->blocksize) {
-                return PyErr_Format(PyExc_ValueError, "uWSGI cache items size must be < %llu, requested %llu bytes", (unsigned long long)uwsgi.caches->blocksize, (unsigned long long) vallen);
+                return PyErr_Format(PyExc_ValueError, "uWSGI cache items size must be < %lu, requested %lu bytes", (unsigned long)uwsgi.caches->blocksize, (unsigned long) vallen);
         }

         if (remote && strlen(remote) > 0) {

@unbit
Copy link
Owner

unbit commented Jan 27, 2013

the only version-independent solution is see is not reporting the size in the exception...

some other solution ?

@prymitive
Copy link
Contributor

Not from me. And grep "%ll" plugins/python/* shows 11 places where such formatting is used, so it's not only those exceptions.

@prymitive
Copy link
Contributor

We could always format those messages before passing them to PyErr_Format()

@unbit
Copy link
Owner

unbit commented Feb 7, 2013

For the caching framework we could safely use %lu as long represent the maximum size of mappable memory (so you cannot have values bigger than 4G on 32bit). I will make a patch for stable branch soon

@unbit
Copy link
Owner

unbit commented May 22, 2013

I close this as error formatting got moved to simple uwsgi_log with the magic api

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants