django-sharding icon indicating copy to clipboard operation
django-sharding copied to clipboard

Inheriting from ShardedByMixing doesn't activate signal.

Open M1ha-Shvn opened this issue 7 years ago • 1 comments

Hey. In my test app I want to shard by ShardingApp model. And split ShardedUser into multiple database shards. Here is the code, I expected to work:

@model_config(database='default')
class ShardingApp(models.Model, ShardedByMixin):
    name = models.CharField(max_length=50, default='Test')

    def get_shard_key(self):
        return self.pk


@model_config(shard_group='shards_by_app_group', sharded_by_field='app_id')
class ShardedUser(models.Model):
    objects = ShardManager()

    id = PostgresShardGeneratedIDAutoField(primary_key=True)
    app_id = models.PositiveIntegerField()

    def get_shard(self):
        return ShardingApp.objects.get(pk=self.app_id).shard

    @staticmethod
    def get_shard_from_id(app_id):
        return ShardingApp.objects.get(pk=app_id).shard

But when I called ShardingApp.objects.create() I got shard = None in the database. The bug is that pre_save signal is called only if shard field has attribute django_sharding__stores_shard = True which is not correct for models.CharField used in ShardedByMixin. I've tried to solve the problem by replacing default CharField with ShardStorageCharField:

@model_config(database='default')
class ShardingApp(BaseModel, ShardedByMixin):
    # Need to redefine it, as choices don't see it during class creation
    SHARD_CHOICES = ((i, i) for i in _get_primary_shards())

    name = models.CharField(max_length=50, default='Test')

    # Here I redeclared shard field.
    shard = ShardStorageCharField(max_length=120, blank=True, null=True, choices=SHARD_CHOICES,
                                  shard_group='shards_by_app_group')

    def get_shard_key(self):
        return self.pk

But this also doesn't work: pre_signal is executed, but it gets ShardingApp model as a sender. And in this line it wants django_sharding__shard_group attribute:

bucketer = apps.get_app_config(app_config_app_label).get_bucketer(sender.django_sharding__shard_group)

So the resulting working code was:

@model_config(database='default')
class ShardingApp(BaseModel, ShardedByMixin):
    # BUG Signal wants it. I suggest adding that in the docs
    django_sharding__shard_group = 'shards_by_app_group'

    SHARD_CHOICES = ((i, i) for i in _get_primary_shards())

    name = models.CharField(max_length=50, default='Test')

    # BUG Signal is not called if it's simple CharField. I suggest replace field in ShardedByMixin.
    shard = ShardStorageCharField(max_length=120, blank=True, null=True, choices=SHARD_CHOICES,
                                  shard_group=django_sharding__shard_group)

    def get_shard_key(self):
        return self.pk

My suggestions are:

  1. Replace CharFIeld with ShardStorageCharField in ShardedByMixin by default
  2. Write in the docs, that django_sharding__shard_group = 'shards_by_app_group' should be defined in ShardedBy model.

M1ha-Shvn avatar Dec 05 '17 06:12 M1ha-Shvn

Thanks for the documentation errors and this, I'll try to get to taking a look this weekend.

JBKahn avatar Dec 05 '17 06:12 JBKahn