ponyorm / pony

Pony Object Relational Mapper
Apache License 2.0
3.65k stars 245 forks source link

TypeError: __init__() got multiple values for argument 'database' with pony 0.7.15 #628

Closed pykler closed 2 years ago

pykler commented 2 years ago

There is a regression with pony 0.7.15 ... when creating a db connection using kwargs the following error is raised:

  File "/opt/venv/lib/python3.8/site-packages/pony/orm/core.py", line 765, in bind
    self._bind(*args, **kwargs)
  File "/opt/venv/lib/python3.8/site-packages/pony/orm/core.py", line 787, in _bind
    self.provider = provider_cls(self, *args, **kwargs)
TypeError: __init__() got multiple values for argument 'database'

the reason seems to be due to self being passed in at pony/orm/core.py:787

self.provider = provider_cls(self, *args, **kwargs)

where self is the argument database which is also being passed in kwargs as the database name as per the docs

import pony.orm
db = pony.orm.Database()
db.bind(provider='postgres', user='', password='', host='', database='')

raises the TypeError mentioned above

note that there is a comment on line 767 that says

# argument 'self' cannot be named 'database', because 'database' can be in kwargs

guess the update to DBAPIProvider didnt adhere to that statement ... this should actually fail if you use database as a kwarg in any provider not just postgres!

astro-josh commented 2 years ago

also seeing this issue in our pipelines, we use **kwargs to init Database object.

pykler commented 2 years ago

The issue was introduced with this commit a few days ago https://github.com/ponyorm/pony/commit/8a6ccb7c6273ca173779cf8993ff05ffbada305e

sashaaero commented 2 years ago

Thanks for reporting. Will fix it asap.

pykler commented 2 years ago

renaming that argument should be it ... there doesnt seem to be any unit tests for that code path though.

sashaaero commented 2 years ago

It's actually not true, we just didn't launch tests for each database provider so this was missed.