Fix compatibility with Peewee ORM

Jamim commented 5 years ago

This patch adds handling for psycopg2.extensions.register_type

The original register_type method checks a type of the conn_or_curs and it doesn't accept wrappers, so we need to make sure that it will not be broken.

Here is a sample traceback:

$ python demo.py 
Traceback (most recent call last):
  File "demo.py", line 18, in <module>
    psql_db.create_tables([DemoModel])
  File "/home/mim/.local/share/virtualenvs/test-peewee-tracing-QFmnjpzE/lib/python3.7/site-packages/peewee.py", line 3036, in create_tables
    model.create_table(**options)
  File "/home/mim/.local/share/virtualenvs/test-peewee-tracing-QFmnjpzE/lib/python3.7/site-packages/peewee.py", line 6056, in create_table
    and cls.table_exists():
  File "/home/mim/.local/share/virtualenvs/test-peewee-tracing-QFmnjpzE/lib/python3.7/site-packages/peewee.py", line 6046, in table_exists
    return cls._schema.database.table_exists(M.table.__name__, M.schema)
  File "/home/mim/.local/share/virtualenvs/test-peewee-tracing-QFmnjpzE/lib/python3.7/site-packages/peewee.py", line 3014, in table_exists
    return table_name in self.get_tables(schema=schema)
  File "/home/mim/.local/share/virtualenvs/test-peewee-tracing-QFmnjpzE/lib/python3.7/site-packages/peewee.py", line 3489, in get_tables
    cursor = self.execute_sql(query, (schema or 'public',))
  File "/home/mim/.local/share/virtualenvs/test-peewee-tracing-QFmnjpzE/lib/python3.7/site-packages/peewee.py", line 2873, in execute_sql
    cursor = self.cursor(commit)
  File "/home/mim/.local/share/virtualenvs/test-peewee-tracing-QFmnjpzE/lib/python3.7/site-packages/peewee.py", line 2859, in cursor
    self.connect()
  File "/home/mim/.local/share/virtualenvs/test-peewee-tracing-QFmnjpzE/lib/python3.7/site-packages/peewee.py", line 2817, in connect
    self._state.set_connection(self._connect())
  File "/home/mim/.local/share/virtualenvs/test-peewee-tracing-QFmnjpzE/lib/python3.7/site-packages/peewee.py", line 3469, in _connect
    pg_extensions.register_type(pg_extensions.UNICODE, conn)
TypeError: argument 2 must be a connection, cursor or None

coveralls commented 5 years ago

Coverage increased (+0.2%) to 76.937% when pulling 9f4f0c227d059776d48171300f99597fe67a1ff1 on Jamim:fix/peewee-orm into f425ad18d747dd6be49b0737a6857191e62b91c6 on uber-common:master.

Jamim commented 5 years ago

Affected code of Peewee ORM: peewee.py#L3293-L3295

Jamim commented 5 years ago

I am not sure what problem this is fixing, could you please expand the description with a sample / stacktrace, etc.?

Unfortunately, I'm not able to share existing stacktraces. I'll prepare some sample and expand the description tomorrow.

Jamim commented 5 years ago

Hello @yurishkuro,

Sorry for the delay. Below you can find the steps to reproduce the issue and to check the proposed solution using a minimal demo app (please use some temporary virtual env):

git clone https://github.com/Jamim/peewee-tracing-demo.git
cd peewee-tracing-demo
docker-compose up -d
pip install -r requirements.txt
python demo.py

# checking of the solution
git checkout fixed
pip install -U -r requirements.txt
python demo.py

Jamim commented 5 years ago

Hi @yurishkuro, Could you please take a look at this PR once again? Thank you!

Jamim commented 5 years ago

You can find a traceback example at the description.

yurishkuro commented 5 years ago

the build is failing consistently

Jamim commented 5 years ago

the build is failing consistently

If you mean errors on CI, then it's the same issue as on the master branch. https://travis-ci.org/uber-common/opentracing-python-instrumentation/branches

opentracing/opentracing-python#120 should fix it.

Jamim commented 5 years ago

Hello @yurishkuro,

Please restart a build on CI to make sure that this PR passes tests now.

Thanks!

yurishkuro commented 5 years ago

restarted (it should pick the new release of opentracing-python)

Jamim commented 5 years ago

Tests are passed :tada:

uber-common / opentracing-python-instrumentation

Fix compatibility with Peewee ORM #72