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

Importing fails or raises error for model that contains primary key which is not id #84

Open
xalien10 opened this issue Dec 19, 2022 · 10 comments

Comments

@xalien10
Copy link
Collaborator

xalien10 commented Dec 19, 2022

I was trying to import a model resource whose primary key is not the id. And when I'm trying to import it from the celery import admin section but it's failing and raising errors. But the same file is able to create/update rows using django-import-export import action.

This is my model resource

class RoleDetailResource(resources.ModelResource):

    class Meta:
        model = RoleDetail
        chunk_size = 5000
        import_id_fields = ("owner", )
        exclude = ("id", )

And I'm getting the following errors while importing,

Error with dry run:
with_dry_run

Error without dry run
without_dry_run

But import was successful with django-import-export action and row can be seen on the list:
import_with_django_import_export

@timthelion

@timthelion
Copy link
Member

Are you using the same resource for both?

@xalien10
Copy link
Collaborator Author

Yes, I'm using same resource for both. In fact, this same resource is used for export also. Export with django-import-export-celery is working fine but import is failing

@timthelion
Copy link
Member

Sorry, but I've read the stacktrace 3 times now and nothing is occurring to me what could be wrong. The only thing that looks like it might be going down a non-standard path is this

[f.column_name for f in resource.get_user_visible_fields()]
try commenting it out to see if it helps.

Sorry I couldn't be of more use.

@xalien10
Copy link
Collaborator Author

xalien10 commented Dec 19, 2022

Could you please tell me which version of celery, django and django-import-export you were using for testing the scenario?

@timthelion
Copy link
Member

Unfortunately, I didn't have the time to actually run a test myself. I only looked over your stack trace. However, you can find exact versions used in production here and here.

@vkgautham
Copy link

vkgautham commented Jan 16, 2023

Import fails in django-import-export-celery for models with foreign key reference to a model that have primary key other than id. But the same is working in django-import-export.

A model foo refers to a model Department (which has primary key other than id)

The resource class for foo is:

class fooResource(resources.ModelResource):
    department = fields.Field(column_name='department', attribute='department', widget=ForeignKeyWidget(Department, 'code'))

    class Meta:
        model = foo

During import in django-import-export the dry run result is
django-import-export

However, when using django-import-export-celery, for the same resource and import file, the following exception is shown:
django-import-export-celery

@timthelion

@vkgautham
Copy link

@timthelion

A quick workaround to this issue, is to change the name of the custom primary key of a foreign key model to id.

@Jaspreet-singh-1032
Copy link
Collaborator

I was looking into this issue and here are some of the things I have found.

The error was being raised from this line.
https://github.com/django-import-export/django-import-export/blob/main/import_export/resources.py#L363

Here is the scenario I used for testing.

class WinnersResource(ModelResource):
    class Meta:
        model = Winner
        import_id_fields = ('name', ) 

    def get_export_queryset(self):
        """To customise the queryset of the model resource with annotation override"""
        return self.Meta.model.objects.all()

In the WinnersResource I set name field in import_id_fields. based on this the method get_import_id_fields should have returned ["name"] but it was still returning ["id"].

The main reason for this behavior is when we are using modelresource_factory function to create model resource here.
https://github.com/auto-mat/django-import-export-celery/blob/master/import_export_celery/model_config.py#L15

I tried to create two resource objects one by using modelresource_factory and other from the WinnersResource class.
image

the r is the object created from WinnersResource class and resource is the object created from modelresource_factory.
You can see the code of modelresource_factory function here.
https://github.com/django-import-export/django-import-export/blob/main/import_export/resources.py#L1228

So, what is happening is when we are using modelresource_factory function then we are not using the Resource class created by us but it creates a resource class by itself and the meta class of that resource does not have attributes defined by us. The only attribute it has is model. That's why it tries to get id which is raising an error because this is the default value for import_id_fields.
https://github.com/django-import-export/django-import-export/blob/main/import_export/resources.py#L76

So, the solution for this issue as I am thinking could be to mention this in the documentation. If the user is defining custom resource classes for models then they should make sure to mention that resource class in settings in IMPORT_EXPORT_CELERY_MODELS. something like this:-

IMPORT_EXPORT_CELERY_MODELS = {
    "Winner": {"app_label": "winners", "model_name": "Winner", "resource": WinnersResource}
}

So, the resource class defined by users should be used and not the resource created by
modelresource_factory.

@xalien10 If you can try this solution and let us know if it fixes this issue then it would be great.

@timthelion let me know if you have any comments here...

@xalien10
Copy link
Collaborator Author

But I was hoping to use before_import method of Resource class to override the import_id_fields
https://github.com/django-import-export/django-import-export/blob/main/import_export/resources.py#L647

Though I'haven't tried it may be we can give it a try by the following approach

class RoleDetailResource(resources.ModelResource):

    class Meta:
        model = RoleDetail
        chunk_size = 5000
        import_id_fields = ("owner", )
        exclude = ("id", )
  
    def before_import(self, dataset, using_transactions, dry_run, **kwargs):
        """
        Override to add additional logic. Does nothing by default.
        """
        self._meta. import_id_fields = ("owner",)

What do you think about this one?

@Jaspreet-singh-1032
@timthelion

@Jaspreet-singh-1032
Copy link
Collaborator

This class will be ignored until you do not mention your resource class in IMPORT_EXPORT_CELERY_MODELS. Because the way code is working here is that if you don't mention your resource class in settings all it will do is create a resource class by itself by using modelresource_factory method. So, whatever changes you will do with your RoleDetailResource will not make any changes.
So if you want your RoleDetailResource to be used by the code then you have to mention it in settings something like this.
https://github.com/auto-mat/django-import-export-celery/blob/master/README.rst?plain=1#L45

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

No branches or pull requests

4 participants