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

Make the filename available to import_data #237

Closed
rhunwicks opened this issue Apr 15, 2015 · 6 comments
Closed

Make the filename available to import_data #237

rhunwicks opened this issue Apr 15, 2015 · 6 comments

Comments

@rhunwicks
Copy link
Contributor

I'd like to be able to use the filename of the uploaded file in the admin to customize the processing - in particular I need to add a column to the dataset to indicate the parent record in a foreign key relationship, based on the name of the file.

In order to do this I propose (and I'm happy to prepare a PR):

We change admin.ImportMixin so that the calls to import_data look like:

    def process_import(self, request, *args, **kwargs):
        '''
        <snip>
        '''
            result = resource.import_data(dataset, dry_run=False,
                                 raise_errors=True, file_name=import_file_name)

    def import_action(self, request, *args, **kwargs):
        '''
        <snip>
        '''
                result = resource.import_data(dataset, dry_run=True,
                                              raise_errors=False, file_name=uploaded_file.name)

In order for this to work, import_data will have to accept the file name. However, I don't think that we should add this parameter to the formal arguments, because it won't be relevant to most users.

I think we should just allow kwargs on import_data so that subclasses of Resource have the required flexibility. I am doing this for other use cases too - for example I have an ImplicitParentResource that can use the provided dataset to import data into a parent model first (using a parent Resource) and then into the actual model used by the Resource where import_data was called. In this case, I need to pass the user from the Request into each Resource so that I can create the LogEntry records (because the default implementation can only do it for the main Resource called from the admin.):

    def import_data(self, dataset, dry_run=False, raise_errors=False,
            use_transactions=None, **kwargs):
@rhunwicks
Copy link
Contributor Author

It would probably also be helpful to pass the **kwargs through from import_data to before_import so that the user can just override before_import in the subclass:

    def before_import(self, dataset, dry_run, **kwargs):
        """
        Override to add additional logic.
        """
        pass

    def import_data(self, dataset, dry_run=False, raise_errors=False,
            use_transactions=None, **kwargs):
        """
        Imports data from ``dataset``.

        ``use_transactions``
            If ``True`` import process will be processed inside transaction.
            If ``dry_run`` is set, or error occurs, transaction will be rolled
            back.
        """
        result = Result()
        result.diff_headers = self.get_diff_headers()

        if use_transactions is None:
            use_transactions = self.get_use_transactions()

        if use_transactions is True:
            # when transactions are used we want to create/update/delete object
            # as transaction will be rolled back if dry_run is set
            real_dry_run = False
            transaction.enter_transaction_management()
            transaction.managed(True)
        else:
            real_dry_run = dry_run

        try:
            self.before_import(dataset, real_dry_run, **kwargs)

@bmihelac
Copy link
Member

Hi @rhunwicks, I like the idea

@rhunwicks
Copy link
Contributor Author

I'll do a PR then.

@rhunwicks
Copy link
Contributor Author

I think I should also pass the request.user in along with the filename from the admin mixins.

bmihelac added a commit that referenced this issue Apr 22, 2015
Fix #237 - make filename available in import_data
@RichardForshaw
Copy link

Hi, I am using this commit and I can't get it working.

I've looked in admin.py and I think that the wrong filename is being passed through to import_data. The code passes through uploaded_file.name:

            with tempfile.NamedTemporaryFile(delete=False) as uploaded_file:
                for chunk in import_file.chunks():
                    uploaded_file.write(chunk)

            # then read the file, using the proper format-specific mode
            with open(uploaded_file.name,
                      input_format.get_read_mode()) as uploaded_import_file:
...<snip>...
                result = resource.import_data(dataset, dry_run=True,
                                              raise_errors=False,
                                              file_name=uploaded_file.name,
                                              user=request.user)

but uploaded_file is the temporary file, so my before_import override is getting this file name rather than the one that the user selected. Shouldn't import_data be passed file_name=import_file.name?

If so, then the ConfirmInputForm form will also need to have an extra hidden field to preserve the original file name so that it can be passed to import_data in ImportMixin.proces_import.

@rhunwicks
Copy link
Contributor Author

Agreed. I'll do a fix.

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

3 participants