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

Dev & Staging v2.5.0 - Exceptions thrown when calling API methods #1498

Closed
jseppi opened this Issue Mar 17, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@jseppi
Contributor

jseppi commented Mar 17, 2017

Looks like a problem with djorm_pgfulltext. Logs below:

2017-03-16T22:54:03.13-0500 [APP/PROC/WEB/0]ERR   File "/home/vcap/app/api/views.py", line 173, in get
2017-03-16T22:54:03.13-0500 [APP/PROC/WEB/0]ERR     contracts_all = self.get_queryset(request.query_params, wage_field)
2017-03-16T22:54:03.13-0500 [APP/PROC/WEB/0]ERR   File "/home/vcap/app/api/views.py", line 205, in get_queryset
2017-03-16T22:54:03.13-0500 [APP/PROC/WEB/0]ERR     return get_contracts_queryset(request, wage_field)
2017-03-16T22:54:03.13-0500 [APP/PROC/WEB/0]ERR   File "/home/vcap/app/api/views.py", line 100, in get_contracts_queryset
2017-03-16T22:54:03.13-0500 [APP/PROC/WEB/0]ERR     contracts = contracts.multi_phrase_search(qs)
2017-03-16T22:54:03.13-0500 [APP/PROC/WEB/0]ERR   File "/home/vcap/app/contracts/models.py", line 136, in multi_phrase_search
2017-03-16T22:54:03.13-0500 [APP/PROC/WEB/0]ERR     return self.search(convert_to_tsquery_union(queries), raw=True)
2017-03-16T22:54:03.13-0500 [APP/PROC/WEB/0]ERR   File "/app/.heroku/python/lib/python3.6/site-packages/djorm_pgfulltext/models.py", line 315, in search
2017-03-16T22:54:03.13-0500 [APP/PROC/WEB/0]ERR     "%s('%s', %s)" % (function, config, adapt(query))
2017-03-16T22:54:03.13-0500 [APP/PROC/WEB/0]ERR   File "/app/.heroku/python/lib/python3.6/site-packages/djorm_pgfulltext/utils.py", line 10, in adapt
2017-03-16T22:54:03.13-0500 [APP/PROC/WEB/0]ERR     a.prepare(connection.connection)
2017-03-16T22:54:03.13-0500 [APP/PROC/WEB/0]ERR TypeError: argument 1 must be psycopg2.extensions.connection, not ConnectionWrapper

2017-03-16T22:54:03.22-0500 [RTR/1]      OUT calc-staging.app.cloud.gov - [2017-03-17T03:54:03.123+0000] "GET /api/rates/?histogram=12&q=te&min_experience=0&max_experience=45&sort=current_price&query_type=match_all&experience_range=0%2C45 HTTP/1.1" 500 0 20535 "https://calc-staging.app.cloud.gov/?q=te" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.98 Safari/537.36" "127.0.0.1:8576" "10.10.3.7:63976" x_forwarded_for:"72.48.211.158, 72.48.211.158, 54.208.160.151" x_forwarded_proto:"https" vcap_request_id:"bcfed5b7-5af3-489f-4be5-134d6d04ce7e" response_time:0.100149665 app_id:"db7756dc-7ca3-417a-aeb2-bb7e0b1e9286" app_index:"0"
@jseppi

This comment has been minimized.

Contributor

jseppi commented Mar 17, 2017

Hrm...I have a theory: I think CloudFoundy is using a cached version of djorm_pgfulltext instead of the forked version we have listed in requirements.txt as a git commit.

I kinda recall some similar weirdness waaay back when this team started on CALC 2.0.

@toolness

This comment has been minimized.

Contributor

toolness commented Mar 17, 2017

Ah shoot!

Hmm, I wonder what is up. Based on the traceback, it does seem like cloudfoundry is using the forked version, as the last line of the traceback is line 10, which matches with 18F/djorm-ext-pgfulltext@1ad8bdd (that commit was made because I was running into an exception where connection.connection was None).

Hmm, I wonder what is causing it though. Hmmmm.

@toolness

This comment has been minimized.

Contributor

toolness commented Mar 17, 2017

OMG i think it might be friggin newrelic!

I found this similar bug that someone was having with a different web framework: https://groups.google.com/forum/#!topic/web2py/nbLhSJhirM4

One of the commenters mentioned:

I've found a solution For this problem. The problem was that for the version Web2py 2.13.4 the newrelic wasnt working. after the i removed the newrelic it worked for me.

More web searching yielded https://gist.github.com/mjallday/3d4c92e7e6805af1e024.

Agggh.

@toolness

This comment has been minimized.

Contributor

toolness commented Mar 17, 2017

Yup, finding that I'm able to reproduce this error by adding fake new relic env vars to my .env file:

NEW_RELIC_LICENSE_KEY=bleh
NEW_RELIC_APP_NAME=whatevs

Arrggg newrelic!!!!!!

Gonna see if I can fix this somehow.

@jseppi

This comment has been minimized.

Contributor

jseppi commented Mar 17, 2017

oh geez

toolness added a commit that referenced this issue Mar 17, 2017

toolness added a commit to 18F/djorm-ext-pgfulltext that referenced this issue Mar 17, 2017

Set encoding of QuotedString instead of calling prepare().
As described in 18F/calc#1498, NewRelic
database instrumentation makes passing a connection to
QuotedString::prepare() problematic.

This commit attempts to work around the problem by setting the
encoding of the QuotedString to the connection's encoding, which
*seems* like a decent workaround, as that looks like most of what
the [QuotedString][] uses the connection for.

The one exception to this is when QuotedString::getquoted() is
called, in which case [psycopg_escape_string][] is called. This
function appears to call [PQescapeString][] if it's passed a
NULL database connection, which according to the Postgres documentation
should work okay as long as we "only work with one PostgreSQL
connection at a time".

[QuotedString]: https://github.com/psycopg/psycopg2/blob/d2cd1236a8638eeee39d78187e76572ed6bfc19d/psycopg/adapter_qstring.c
[psycopg_escape_string]: https://github.com/psycopg/psycopg2/blob/f9b36433fb0103f0e98fe3d579103e0352667315/psycopg/utils.c
[PQescapeString]: https://www.postgresql.org/docs/9.6/static/libpq-exec.html

toolness added a commit that referenced this issue Mar 17, 2017

Alternative fix for #1498.
Most of the fix is actually contained in our
`djorm-ext-pgfulltext` fork, which this PR references in its
`requirements.txt`. For more details, see:

    18F/djorm-ext-pgfulltext#1

This PR also contains regression tests to ensure that the
fix works.

toolness added a commit that referenced this issue Mar 17, 2017

toolness added a commit that referenced this issue Mar 20, 2017

Merge pull request #1502 from 18F/another-alternate-fix-for-1498
Yet another alternate fix for #1498 (unwrapping approach)
@jseppi

This comment has been minimized.

Contributor

jseppi commented Mar 20, 2017

Closed by #1502

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment