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

Provided docstrings for client.py #11

Merged
merged 18 commits into from
Nov 13, 2013
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions KISSmetrics/__init__.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,22 @@
# -*- coding: utf-8 -*-

#: Current URI for all Tracking service endpoints.
Copy link
Contributor

Choose a reason for hiding this comment

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

Default host for tracking service endpoint

TRACKING_HOSTNAME = 'trk.kissmetrics.com'
#: Default protocol for inbound request to the Tracking service.
Copy link
Contributor

Choose a reason for hiding this comment

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

Default protocol for tracking service requests

Copy link
Contributor

Choose a reason for hiding this comment

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

per documentation and RFCs i just read... protocol is scheme in this case

TRACKING_PROTOCOL = 'http'

#: The URI for the endpoint to record events via the Tracking service.
Copy link
Contributor

Choose a reason for hiding this comment

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

"URI to record events via the tracking service" maybe? I generally try to omit articles (i.e. "the", "a"/"an") from my documentation since they are pretty low on information value. Same goes for the following few constants.

Copy link
Contributor

Choose a reason for hiding this comment

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

we should call these paths if we want to be correct. we can let the docs fly through like this and I'm happy to do a pass through cleaning up our naming conventions afterwards

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. I'm curious, is there any good reason to actually expose these? In other words, is there a legitimate scenario where a user would want to send an event request to somewhere other than /e? If not I suggest we eliminate the uri (path) parameters from the client methods. I'm working on some misc. refactoring and can bundle that together.

Copy link
Contributor

Choose a reason for hiding this comment

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

two fold. test coverage and the fact that we DO support multiple environments such as our staging environment and a pilot program which will offer dedicated infrastructure for very large customers.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha. I figured there was a use case in there somewhere, thanks.

#:
#: :see: http://support.kissmetrics.com/apis/specifications#recording-an-event
RECORD_URI = 'e'
#: The URI for the endpoint to set a property via the Tracking service.
#:
#: :see: http://support.kissmetrics.com/apis/specifications#setting-properties
SET_URI = 's'
#: The URI for the endpoint that aliases identities performing events via the
#: Tracking service.
#:
#: :see: http://support.kissmetrics.com/apis/specifications#aliasing-users
ALIAS_URI = 'a'

__author__ = 'Ernest W. Durbin III <ewdurbin@gmail.com>'
Expand Down
73 changes: 73 additions & 0 deletions KISSmetrics/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,22 @@


class Client:
"""Interface to KISSmetrics tracking service."""
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW (and there is debate about this) I usually ditch the period from the first line of my class docstrings too. My rule of thumb is that if I have no verb, I keep things period-free.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done - 90a8b01


def __init__(self, key, trk_host=KISSmetrics.TRACKING_HOSTNAME,
trk_proto=KISSmetrics.TRACKING_PROTOCOL):
"""Initialize client for use with KISSmetrics API key.

:param key: the API key is found on the "KISSmetrics Settings"
for a product.
:type key: str

:param trk_host: tracking URL for all requests; defaults
production tracking service.
:param trk_proto: the protocol for requests; either be `'http'`
or `'https'`.

"""
self.key = key
self.trk_host = trk_host
if trk_proto not in ['http', 'https']:
Expand All @@ -21,6 +34,21 @@ def url(self, query_string):

def record(self, person, event, properties={}, timestamp=None,
uri=KISSmetrics.RECORD_URI):
"""Records `event` for `person` with any `properties`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Record or Records?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

corrected: d1d4909


:param person: the individual performing the `event`
:param event: the `event` name that was performed
:param properties: any additional data to include
:type properties: dict
:param timestamp: when the `event` was performed; optional for
back-dating
:param uri: HTTP endpoint to use; defaults to
``KISSmetrics.RECORD_URI``

:returns: an HTTP response for the request
:rtype: `urllib3.response.HTTPResponse`

"""
this_request = request.record(self.key, person, event,
timestamp=timestamp,
properties=properties, uri=uri)
Expand All @@ -29,12 +57,57 @@ def record(self, person, event, properties={}, timestamp=None,

def set(self, person, properties={}, timestamp=None,
uri=KISSmetrics.SET_URI):
"""Sets a property (or properties) for a `person`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Set or Sets?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

corrected: d1d4909


:param person: the individual to associated properties with
:param properties: key-value pairs to associated with `person`
:type properties: dict
:param timestamp: when the `event` was performed; optional for
back-dating
:param uri: HTTP endpoint to use; defaults to
``KISSmetrics.SET_URI``

:returns: an HTTP response for the request
:rtype: `urllib3.response.HTTPResponse`

"""
this_request = request.set(self.key, person, timestamp=timestamp,
properties=properties, uri=uri)
url = self.url(this_request)
return self.http.request('GET', url)

def alias(self, person, identity, uri=KISSmetrics.ALIAS_URI):
"""Maps `person` to `identity`; actions done by one resolve the other.
Copy link
Contributor

Choose a reason for hiding this comment

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

Map or Maps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

corrected: d1d4909


:param person: consider as same individual ``identity``; the
source of the alias operation
:type person: str or unicode
:param identity: consider as an alias of ``person``; the target
of the alias operation
:type identity: str or unicode
:param uri: HTTP endpoint to use; defaults to
``KISSmetrics.ALIAS_URI``

:returns: an HTTP response for the request
:rtype: `urllib3.response.HTTPResponse`

Note the direction of the mapping is ``person`` to ``identity``
(so ``person`` is also known as ``identity`` or ``person`` =>
``identity`` when looking at it as "source => target")

When consulting the Aliasing documentation, `person` corresponds
to ``query_string.PERSON_PARAM`` and `identity` corresponds to
``query_string.ALIAS_PARAM``.

Aliasing is not a reversible operation. When aliasing to an
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this documentation 👍

identity, take care not to use a session identifier or any other
value that is not relatively stable (a value that will not
change per request or per session).

For more information see the API Specifications on `Aliasing
<http://support.kissmetrics.com/apis/specifications.html#aliasing-users>`_.

"""
this_request = request.alias(self.key, person, identity, uri=uri)
url = self.url(this_request)
return self.http.request('GET', url)