Skip to content
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

Inheriting from ShardedByMixing doesn't activate signal. #70

Open
M1ha-Shvn opened this issue Dec 5, 2017 · 1 comment
Open

Inheriting from ShardedByMixing doesn't activate signal. #70

M1ha-Shvn opened this issue Dec 5, 2017 · 1 comment

Comments

@M1ha-Shvn
Copy link

M1ha-Shvn commented Dec 5, 2017

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.
@JBKahn
Copy link
Owner

JBKahn commented Dec 5, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants