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

Fix #929 - Make Error management more DRY #958

Merged
merged 7 commits into from
Apr 15, 2016

Conversation

deepthivenkat
Copy link
Member

No description provided.

429: 'Cool your jets! Please wait {0} seconds before making'
'another search.',
500: 'Internal Server Error',
GitHubError: 'Something bad happened. Please try again?'}

This comment was marked as abuse.

@karlcow
Copy link
Member

karlcow commented Mar 11, 2016

@deepthivenkat

Oh and why the additional pull request here? You already started #949 which was not finished.

@deepthivenkat
Copy link
Member Author

I would like to continue with this pull request as I have deleted the repository of the pull request #949

@deepthivenkat
Copy link
Member Author

@karlcow when I read the code, I realized that GithubError is a class that is getting imported from flask.ext.github.

I also read from this link http://stackoverflow.com/questions/7560172/class-as-dictionary-key-in-python that class can be used as a key for Dictionary.

I also executed this code snippet in python interpreter before submitting the pull request :

class MyError(Exception):
        def dee_excep_():
            try:
                raise MyError(2*2)
            except MyError as e:
                return e.value
            dee_excep_()



ERROR_DICT = { 400: 'Bad Request.', 
               401: 'Unauthorized. Please log in.',
               403: 'Forbidden. Maybe that looking at private stuff?', 
           404: 'Not Found. Lost in Punk Cat Space',
               429: 'Cool your jets! Please wait {0} seconds before making'
               ' another search.',
               500: 'Internal Server Error',
               MyError: "'Something bad happened. Please try again?', 'error'"}

print ERROR_DICT[MyError]
print type(ERROR_DICT[MyError])

This executed without any errors. I did make built which did not return any errors. If this is causing the build to fail, I will change the dictionary key from class object to string.

@karlcow
Copy link
Member

karlcow commented Mar 14, 2016

So currently https://travis-ci.org/webcompat/webcompat.com/builds/115635958#L573
all the unit tests about errors are failing. We need to understand why.

@karlcow
Copy link
Member

karlcow commented Mar 14, 2016

@deepthivenkat You need to run nosetests in your own directory and you will see the error messages.

@karlcow karlcow changed the title Issue#929 Fix #929 - Make Error management more DRY Mar 14, 2016
@@ -61,11 +61,6 @@ def test_activity_page_401_if_not_logged_in(self):
rv = self.app.get('/me')
self.assertEqual(rv.status_code, 401)

def test_activity_page_403_view_other_user_activity(self):

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

@@ -44,8 +43,7 @@
404: 'Not Found. Lost in Punk Cat Space',
429: 'Cool your jets! Please wait {0} seconds before making'
'another search.',

This comment was marked as abuse.

@karlcow
Copy link
Member

karlcow commented Apr 14, 2016

ok let's see

→ git pull https://github.com/deepthivenkat/webcompat.com.git Issue#929
remote: Counting objects: 8, done.
remote: Compressing objects: 100% (8/8), done.
remote: Total 8 (delta 3), reused 0 (delta 0), pack-reused 0
Unpacking objects: 100% (8/8), done.
From https://github.com/deepthivenkat/webcompat.com
 * branch            Issue#929  -> FETCH_HEAD
Merge made by the 'recursive' strategy.
 tests/test_urls.py |   1 +
 webcompat/views.py | 119 +++++++++++++++++++++++++++++++++++++----------------------------------------------------------------------------------
 2 files changed, 38 insertions(+), 82 deletions(-)


(webcompatcom)16:40:13 ~/code/webcompat.com
→ git status
On branch deepthivenkat-Issue#929
nothing to commit, working directory clean


(webcompatcom)16:40:21 ~/code/webcompat.com
→ python run.py 
secrets /Users/karl/code/webcompat.com
 * Running on http://localhost:5000/ (Press CTRL+C to quit)
 * Restarting with stat
secrets /Users/karl/code/webcompat.com
127.0.0.1 - - [14/Apr/2016 16:41:07] "GET /api/jhkjhfkk HTTP/1.1" 404 -

then

→ http GET http://localhost:5000/api/jhkjhfkk 'Accept:application/json'
HTTP/1.0 404 NOT FOUND
Content-Length: 70
Content-Type: application/json
Date: Thu, 14 Apr 2016 07:41:26 GMT
Server: Werkzeug/0.10.4 Python/2.7.10

{
    "message": "Not Found. Lost in Punk Cat Space", 
    "status": 404
}

Good! r+
Thanks a lot @deepthivenkat 🎉

@karlcow
Copy link
Member

karlcow commented Apr 14, 2016

Ahaha. One small thing to fix.
webcompat/views.py:297:1: E302 expected 2 blank lines, found 1

@karlcow
Copy link
Member

karlcow commented Apr 15, 2016

So when I run the tests locally I have no issues.

→ nosetests
........................................
----------------------------------------------------------------------
Ran 40 tests in 4.773s

OK

but travis seems to have issues. Let's see
https://travis-ci.org/webcompat/webcompat.com/builds/122993115

ERROR: Test that we are receiving the appropriate text file.
FAIL: API issue for a non existent number returns JSON 404.
FAIL: API access to labels without auth returns JSON 200.

So there is something to explore here and understands why it doesn't fail locally but fails on travis.

For example

======================================================================

ERROR: Test that we are receiving the appropriate text file.

----------------------------------------------------------------------

Traceback (most recent call last):

  File "/home/travis/build/webcompat/webcompat.com/tests/test_urls.py", line 128, in test_rate_limit

    rv = self.app.get('/rate_limit')



(… CUTTING STUFF…)

  File "/home/travis/build/webcompat/webcompat.com/webcompat/views.py", line 253, in show_rate_limit

    return (render_template('ratelimit.txt', rl=rl), 200,

  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/flask/templating.py", line 128, in render_template

    context, ctx.app)

  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/flask/templating.py", line 110, in _render

    rv = template.render(context)

  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/jinja2/environment.py", line 989, in render

    return self.environment.handle_exception(exc_info, True)

  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/jinja2/environment.py", line 754, in handle_exception

    reraise(exc_type, exc_value, tb)

  File "/home/travis/build/webcompat/webcompat.com/webcompat/templates/ratelimit.txt", line 3, in top-level template code

    Core resources rate limit: {{ rl.resources.core.remaining }} / {{ rl.resources.core.limit }} requests.

  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/jinja2/environment.py", line 408, in getattr

    return getattr(obj, attribute)

UndefinedError: 'dict object' has no attribute 'resources'

-------------------- >> begin captured logging << --------------------

requests.packages.urllib3.connectionpool: INFO: Starting new HTTPS connection (1): api.github.com

requests.packages.urllib3.connectionpool: DEBUG: "GET /rate_limit HTTP/1.1" 401 83

--------------------- >> end captured logging << ---------------------

but if I do:

→ http GET http://localhost:5000/rate_limit 'Accept:application/json'

I get the right thing

HTTP/1.0 200 OK
Content-Length: 155
Date: Fri, 15 Apr 2016 04:17:09 GMT
Server: Werkzeug/0.10.4 Python/2.7.10
content-type: text/plain

Current user: webcompat-bot

Core resources rate limit: 4998 / 5000 requests.
Reset in 48 minutes.

Search rate limit: 30 / 30 requests.
Reset in 1 minute.

hmmm let's see. Maybe just a Travis thing.

@karlcow karlcow merged commit 6269b77 into webcompat:master Apr 15, 2016
@miketaylr
Copy link
Member

Maybe just a Travis thing.

Yeah, it's a Travis thing. It runs into rate limiting probably because it has a shared Travis IP (this is only for tests that make unauthed APy I calls, from a PR from someone who doesn't have access to the secrets. Basically anyone not sending a PR from the webcompat repo itself). On my backlog of things to fix. 🙈

@miketaylr
Copy link
Member

Great work @deepthivenkat!

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.

3 participants