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

Google Analytics Event Tracking for CKAN API #6

Merged
merged 6 commits into from Nov 5, 2014

Conversation

maxious
Copy link
Member

@maxious maxious commented Oct 20, 2013

Intercept some CKAN API routes (not autocomplete, i18n etc.) and send a page view and event to Google Analytics via the new Measurement Protocol API https://developers.google.com/analytics/devguides/collection/protocol/v1/

of the CKAN API can be reported on via Google Analytics.
@rufuspollock
Copy link
Member

@seanh @rossjones @dread who might review this?

@@ -2,6 +2,19 @@
from ckan.lib.base import BaseController, c, render
import dbutil

import urllib
from pprint import pprint
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused?

@rossjones
Copy link
Contributor

I guess a run through PEP8 wouldn't hurt either (I don't care but travis does, or at least it did). At least for me, I'd create the threads up-front and feed them via an internal queue, but perhaps it would be worth measuring thread-creation under load. Am sure this'll work in most use-cases though.

CKAN data.gov.au and others added 2 commits May 22, 2014 07:06
@rufuspollock
Copy link
Member

@maxious is anything holding you up from responding to @rossjones review? I imagine ti would be nice to get this merged if it was satisfactory ...

@davidread any thoughts at your end?

@davidread
Copy link
Contributor

These are useful changes - if @maxious could tidy up the odd thing suggested by the comments we can merge it.

@Starl3n
Copy link
Member

Starl3n commented Oct 31, 2014

Heya folks, We'll have this scheduled as part of @maxious 's day job :) Should get things sorted soon. We might get Sergey to do the work, but either way we'll take this on very soon and get it done :)

Feel free to ping me for any other pull requests so that I can schedule work under our CKAN Association gold membership agreement. @maxious kinda has two hats but where its work that Link Digital can take on then you can certainly feel free to put the pressure on me :)

@maxious
Copy link
Member Author

maxious commented Nov 5, 2014

@rgrp @davidread Okay, actioned feedback + PEP8 compliance.

@rufuspollock
Copy link
Member

@davidread @rossjones is one of you able to review this? (I'm almost certainly not the right person :-) ...)

@davidread
Copy link
Contributor

I like it. @rossjones ?

@rossjones
Copy link
Contributor

Might be useful to allow people to turn it off if they wish? I didn't miss it did I?

Other than that, looks okay.

@davidread
Copy link
Contributor

@rossjones I guess it's easy enough for someone to add config for turning it off, should this be necessary. Merging.

davidread pushed a commit that referenced this pull request Nov 5, 2014
Google Analytics Event Tracking for CKAN API
@davidread davidread merged commit 0febe11 into ckan:master Nov 5, 2014
@davidread
Copy link
Contributor

Thanks @maxious and @Starl3n !

@Starl3n
Copy link
Member

Starl3n commented Nov 5, 2014

hah! I love it when I get credit for doing nothing ;)

This is all work from Maxious. I'm pretty sure he hasn't used and 'day job'
time on this as yet.

Anyhoo - its handy to have stats connected to the CKAN platform requests so
it is good that this can be available to others :)

Cheers,
Steven

_STEVEN DE COSTA *|
*EXECUTIVE DIRECTOR_www.linkdigital.com.au

On 5 November 2014 22:38, David Read notifications@github.com wrote:

Thanks @maxious https://github.com/maxious and @Starl3n
https://github.com/Starl3n !


Reply to this email directly or view it on GitHub
#6 (comment)
.

nigelbabu added a commit that referenced this pull request Feb 13, 2015
…king"

This reverts commit 0febe11, reversing
changes made to e45d73d.

This has been reverted for caushing Apache to max out threads when running
under mod_wsgi.
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

5 participants