Error updating model with a OneToMany relationship
thedamnedrhino opened this issue · 6 comments
UPDATE I am encountering the same issue with deleting the model. I am only calling delete()
on the root model (Campaign
) since I have cascade='delete'
on the child model (CampaignAd
).
I'm trying to update a model (by changing one of its (boolean) properties and calling save()
) on it. However I am encountering an error when the _from_redis()
method executes for its OneToMany property, i.e: I am getting this error:
def _from_redis(self, value): convert = self._allowed if callable(self._allowed) else self._allowed[0] return convert(value)
E TypeError: object() takes no parameters
It seems like convert(value)
is not working because it takes no parameters. self._allowed
is a tuple containing a rom.columns.DoesntMatterInThisContext
class and it seems like its constructor does not accept any arguments and hence the error. (value
is ''
). The OneToMany
relationship in my models are as follows. Also, I'm using the base model classes as mixins.
class Campaign(BaseCampaign, rom.Model):
zakhar_id = rom.PrimaryKey()
id = rom.Text(unique=True, index=True, keygen=rom.IDENTITY)
ads = rom.OneToMany(ftable='CampaignAd', column='campaign')
...
def __init__(self, id: str = '',
ads: List[dsp.CampaignAd] = [],
**kwargs):
rom.Model.__init__(self, id=id, ads=ads,
**kwargs)
class CampaignAd(BaseCampaignAd, rom.Model):
zakhar_id = rom.PrimaryKey()
campaign = rom.ManyToOne(ftable='Campaign', on_delete='cascade')
# the CampaignAd class also has a separate OneToOne relationship to another model
def __init__(self, campaign: Optional['Campaign'] = None, **kwargs):
rom.Model.__init__(self, campaign=campaign, **kwargs)
BaseCampaignAd.__init__(self, campaign)
def set_campaign(self, c: Campaign):
self.campaign = c
It seems like
convert(value)
is not working because it takes no parameters.self._allowed
is a tuple containing arom.columns.DoesntMatterInThisContext
class and it seems like its constructor does not accept any arguments and hence the error. (value
is''
).
All of it matters in this context, as well as a full traceback.
The
OneToMany
relationship in my models are as follows. Also, I'm using the base model classes as mixins.
As per our previous discussion, there are no guarantees with rom being used as a mixin.
I don't know how to help you with this because I don't know what column this is, and I don't know what class this is calling, and without those references or code representing your class, I can't help you. If you want help, you need to provide a traceback and you need to show me more information about your column class. If you don't want to make that public, cool; close this bug and email me. My email is public on my profile.
Dear Josiah, the column in question is the ads
column of the Campaign
model. Below you can find a more complete traceback:
flask_app/dsp.py:107: in set_campaign_enabled
campaign.save(full=True)
venv/lib/python3.6/site-packages/rom/model.py:486: in save
self._last, new, full or self._new or force, is_new=self._new or force)
venv/lib/python3.6/site-packages/rom/model.py:337: in _apply_changes
oval = ca._from_redis(roval) if roval is not None else None
venv/lib/python3.6/site-packages/rom/columns.py:296: TypeError
self = <rom.columns.OneToMany object at 0x11075c818>, value = ''
def _from_redis(self, value):
convert = self._allowed if callable(self._allowed) else self._allowed[0]
return convert(value)
E TypeError: object() takes no parameters
This is the relevant part of the traceback. I should note that I make no change on the Campaign.ads
property before calling save()
on the campaign. All I do is retrieve the Campaign
object from redis. Via the Model.query.all()
method. If you need any other information please let me know
I'm trying out some stuff and I've found out the following. If I delete the model (Campaign
) without retrieving it from redis it will work i.e:
campaign.save()
campaign.delete() # this works
but if I retrieve it from redis I'll get the forementioned error: i.e:
campaign.save()
c = Campaign.query.... # retrieve campaign from redis
assert c is campaign
campaign.delete() # this will get the error mentioned before
I'm getting the same result even without using the model class as a mixin. I also didn't assign anything to the Campaign.ads
property. But I'm still getting the same error. It seems like the error stems from two sources:
- after loading a
Campaign
object from redis (e.g:Campaign.query.all()[0]), its
_lastproperty will be set to a dictionary with the
adskey (key representing the OneToMany relationship) set to
''`. - The
DoesntMatterInThisContext
object does not accept any arguments when it's called.
I would really appreciate it if you could provide some feedback on this soon. Thanks a bunch :))
I found the issue. It was because of overriding the constructor. I shouldn't have passed the OneToMany relationship (ads
in Campaign.__init__
) to the constructor of the rom.Model
class. I think this caused a problem because I had also set a default for it. I think the safest way to override the constructor is to define the signature as __init__(**kwargs)
.
Sorry I'm just getting to this now, I spent the weekend sick with my kids, and am just catching up. Just to make sure I understand the issue and fix (if someone comes back with mixin / overriding / etc. issues in the future):
Campaign()
initialization (on load) was failing because you were trying to pass a list of ads into the Campaign(ads=...)
initialization call.
For others, in the context of rom, this makes sense; Campaign.ads
, and any other OneToMany()
columns don't actually exist in the context of the of the model in which they are defined! Instead, the data is actually stored inside the referencing CampaignAd.campaign
/ ManyToOne()
column; and indexed as a Redis sorted set inside the Redis key CampaignAd:ads:idx
. When referencing Campaign.ads
, that index data is fetched via CampaignAd.get_by(campaign=Campaign.<primary key>)
on attribute reference (the list is not cached, though individual entities may be cached in your session if you haven't disabled session caching). For reference, here's the column 'get' for OneToMany()
: https://github.com/josiahcarlson/rom/blob/master/rom/columns.py#L935 .
This is very similar to how most/all ORMs / SQL databases I've looked at handle the same thing (index the thing from the other side, fetch the list via "magic" index lookup). And yeah, it can definitely be confusing if you're trying to pass stuff in. I haven't tried it recently, so I don't know if SQLAlchemy or other ORMs allow passing lists in the constructor like that.