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

Statistics refactory #120

Closed
wants to merge 36 commits into from
Closed

Statistics refactory #120

wants to merge 36 commits into from

Conversation

henriquebastos
Copy link
Contributor

This PR:

  1. Eliminates custom SQLs for stats;
  2. Reduces stats from 16 queries to 5. (middleware+stats are now 7 in total);
  3. Adds tests for every stats data;
  4. Uses django-aggregate-if to simplify queries;

@tonylampada
Copy link
Member

Thanks HB!
Please bear with me as I won't be able to look into this until I come back from vacation on jan-3.

Stats shows raw history numbers. We don't need to 
filter expired offers.
@tonylampada
Copy link
Member

Thanks and congratulations for creating django-aggregate-if :-)
And nice blog post too.

I still can't get it to work on my machine though. When I try to open /core/stats I bump into a 500:
I'm using postgres, maybe it has something to do with it?

Environment:


Request Method: GET
Request URL: http://localhost:8000/core/stats/

Django Version: 1.4
Python Version: 2.7.3
Installed Applications:
('django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.sites',
 'django.contrib.messages',
 'django.contrib.staticfiles',
 'django.contrib.admin',
 'django.contrib.admindocs',
 'core',
 'core_splinter_tests',
 'gh_frespo_integration',
 'pagination',
 'social_auth',
 'mailer',
 'south',
 'emailmgr',
 'registration')
Installed Middleware:
('django.middleware.common.CommonMiddleware',
 'django.middleware.transaction.TransactionMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware',
 'core.middlewares.CompleteRegistrationFirst',
 'pagination.middleware.PaginationMiddleware')


Traceback:
File "/home/tony/work/solo/www.freedomsponsors.org/local/lib/python2.7/site-packages/django/core/handlers/base.py" in get_response
  111.                         response = callback(request, *callback_args, **callback_kwargs)
File "/home/tony/work/solo/www.freedomsponsors.org/djangoproject/core/views/main_views.py" in stats
  69.     stats = stats_services.get_stats()
File "/home/tony/work/solo/www.freedomsponsors.org/djangoproject/core/services/stats_services.py" in get_stats
  45.     stats.update(get_issue_stats())
File "/home/tony/work/solo/www.freedomsponsors.org/djangoproject/core/services/stats_services.py" in get_issue_stats
  28.         issue_count_sponsoring=Count('pk', only=Q(is_public_suggestion=False)),
File "/home/tony/work/solo/www.freedomsponsors.org/local/lib/python2.7/site-packages/django/db/models/query.py" in aggregate
  338.         return query.get_aggregation(using=self.db)
File "/home/tony/work/solo/www.freedomsponsors.org/local/lib/python2.7/site-packages/django/db/models/sql/query.py" in get_aggregation
  384.         result = query.get_compiler(using).execute_sql(SINGLE)
File "/home/tony/work/solo/www.freedomsponsors.org/local/lib/python2.7/site-packages/django/db/models/sql/compiler.py" in execute_sql
  818.         cursor.execute(sql, params)
File "/home/tony/work/solo/www.freedomsponsors.org/local/lib/python2.7/site-packages/django/db/backends/util.py" in execute
  40.             return self.cursor.execute(sql, params)
File "/home/tony/work/solo/www.freedomsponsors.org/local/lib/python2.7/site-packages/django/db/backends/postgresql_psycopg2/base.py" in execute
  52.             return self.cursor.execute(query, args)

Exception Type: DatabaseError at /core/stats/
Exception Value: operator does not exist: boolean = integer
LINE 1: ...UNT(CASE WHEN "core_issue"."is_public_suggestion" = 0  THEN ...
                                                             ^
HINT:  No operator matches the given name and argument type(s). You might need to add explicit type casts.

@henriquebastos
Copy link
Contributor Author

Which Postgres version are you using? As I remember, all the tests passed
here before the PR.

I will test it again.

@iurisilvio
Copy link
Contributor

@henriquebastos You don't have tests with bool fields. =)

I send a PR in django-aggregate-if project to solve this postgres issue. I'm using PostgreSQL 9.2.

@henriquebastos
Copy link
Contributor Author

Cool! I'll merge it asap

Henrique Bastos http://henriquebastos.net

Curso Welcome to the Django http://welcometothedjango.com.br
Aprenda Python e Django na Prática!

Twitter: @henriquebastos http://twitter.com/henriquebastos
Skype: henriquebastos.net
+55 21 9618-6180

On Mon, Jan 7, 2013 at 9:46 AM, Iuri de Silvio notifications@github.comwrote:

@henriquebastos https://github.com/henriquebastos You don't have tests
with bool fields. =)

I send a PR in django-aggregate-if project to solve this postgres issue.
I'm using PostgreSQL 9.2.


Reply to this email directly or view it on GitHubhttps://github.com//pull/120#issuecomment-11948931.

@henriquebastos
Copy link
Contributor Author

This problem happened due to a bug in django-aggregate-if 0.2. Thanks to @iurisilvio it's fixed! \o/

I've already uploaded version 0.3 to pypi and updated FS requirements.txt.

@tonylampada please try it again.

@tonylampada
Copy link
Member

Can one of the admins verify this patch?

@iurisilvio
Copy link
Contributor

It is probably a bit outdated, but long time ago I tested this pull request and it worked fine.

At least it has a lot of tests, so it is a big improvement to stats module.

@tonylampada
Copy link
Member

Can one of the admins verify this patch?

2 similar comments
@tonylampada
Copy link
Member

Can one of the admins verify this patch?

@tonylampada
Copy link
Member

Can one of the admins verify this patch?

@henriquebastos henriquebastos closed this by deleting the head repository Nov 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants