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

DW Traffic API #1

Closed
wants to merge 4 commits into from
Closed

DW Traffic API #1

wants to merge 4 commits into from

Conversation

danwagnerco
Copy link
Owner

Hey @pavelgrib -- wanted to review a bit of work I did this afternoon and evening on the SDK. In a nutshell:

  1. I'm thinking that we break each high-level API into its own class, meaning SimilarwebTrafficClient.visits, SimilarwebTrafficClient.traffic # <~ to be implemented, SimilarwebTrafficClient.pageviews # <~ to be implemented, etc...
  2. A functools refactoring will probably come in handy as I implement the traffic, pageviews, visitduration and bouncerate methods.
  3. I got around all that test mocking wildness by delegating the response parsing to SimilarwebTrafficClient._parse_visits_response, which actually made testing a cinch. Pythonic or headed down the wrong street?

What do you think -- ready for a merge and further implementation?

@gribml
Copy link

gribml commented Feb 11, 2015

ok, that makes sense... the way I understand your reasoning is as follows:

-you create an object of type SimilarwebClient

  • from there, you call client.traffic() which returns an object you can query the traffic APi
  • or client.content(), which gives you ability to call the content API
    ... etc...

am I understanding this correctly?

Pythonic usually means not too heavy an inheritance structure, this is fine though.

With regards to testing, maybe it makes sense to research the 'responses' package or to get a better grip of the 'mock' package. I think calling _parse_visits_response doesn't actually conduct the unit test... no? one of the tests you would want to do is 'assert mockObj.method.called_exactly_once('requests.get')' which would fail.

@danwagnerco
Copy link
Owner Author

Yo @pavelgrib -- great feedback, I can see how my notes above were unclear.

1. Objects

Based on what I've pulled together so far, there will actually by 4 object types:

  • SimilarwebTrafficClient
  • SimilarwebContentClient
  • SimilarwebMobileClient
  • SimilarwebSourcesClient

Each client object would line up with Similarweb's API grouping, each of which shares a similar-ish URL structure:

image

I was envisioning a consumer of this SDK instantiating client objects as necessary. Suppose the consumer only needs the info from Similarweb's Web Traffic APIs > Traffic API:

from similarweb import SimilarwebTrafficClient

client = SimilarwebTrafficClient("api_key")
d = client.visits("example.com", "monthly", "11-2014", "12-2014", False)
print(d) # => {"11-2014": 888888, "12-2014": 999999}

Is that design philosophy valid, or should we go down a different avenue?

2. Testing

You're right, there are DEFINITELY some holes here.

I'm going to take a step back from the implementation specifics for a minute and put my "business" hat on. The end game is to greatly simplify the Similarweb data collection process for Python programmers. With that in mind, I envision a 4-step process:

  1. Get inputs from user (programmer)
  2. HTTP GET the Similarweb API
  3. Parse Similarweb API response data
  4. Return easy-to-digest data to user (programmer)

With that in-hand, I'll put my Python hat back on.

Fortunately, we don't have to test (1) -- if the programmer is not calling methods correctly, he or she will get an error from Python. One down, three to go.

Interacting with the Similarweb API is made easy by the API's design -- HTTP GET (2). And fortunately the requests package handles HTTP stuff like gangbusters, so the only thing that really needs to be tested is the logic for forming the URL, right? That's something that should be on my TODO list.

Argh -- parsing the HTTP response body from the Similarweb API and returning it is logic-heavy, so these two (3 and 4) needs lots of tests. I tried to isolate these steps by moving all parse logic into the _parse_visists_response method, which takes a JSON-formatted string (which is what we get back from the Similarweb API) as input. I can see how unittest.mock seems like it was designed for this, but I just don't think I'm smart enough to implement it. How would I use something like MagicMock to mock out requests.get(full_url) on line 15 of the similarweb.py file?

@danwagnerco danwagnerco closed this Mar 2, 2015
Repository owner locked and limited conversation to collaborators Mar 2, 2015
@danwagnerco danwagnerco deleted the dw-traffic-api branch March 4, 2015 23:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants