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

Additional customization hooks #61

Closed
rhunwicks opened this issue Jan 20, 2014 · 3 comments
Closed

Additional customization hooks #61

rhunwicks opened this issue Jan 20, 2014 · 3 comments

Comments

@rhunwicks
Copy link
Contributor

There are some hooks in Resource to allow easy customization: before_save_instance, etc.

I want to take an extract file and use it to create some parent models as part of the same import process. The best way for me to do this, is to call the import_data method for the other ModelResources immediately before I start processing the rows (so that I am already inside the transaction management, if required). At the moment, I have to copy and paste the entire import_data method into my Resource: it would be better if we had something like:

        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

        instance_loader = self._meta.instance_loader_class(self, dataset)

        self.before_import(dataset, real_dry_run)

        for row in dataset.dict:

Similarly, some of my customizations work by overriding import_obj, but this can more difficult than necessary because we don't know whether we are in a dry run or not.

This could be solved by adding dry_run to the method signature of import_obj and passing it when you call it from import_data.

Would those changes be acceptable? If they would, I'll prepare a pull request.

@bmihelac
Copy link
Member

It looks like nice addition so please go ahead and make PR. Do not forget tests and docs :)

Maybe transferring dry_run around looks cumbersome, but do not have better idea at the moment.

@rhunwicks
Copy link
Contributor Author

I think that transferring dry_run is the best way for now, because if we are using transaction management we need to pass it as False to the lower-level code even if it is True at the top-level.

@bmihelac
Copy link
Member

agreed

bmihelac added a commit that referenced this issue Jan 21, 2014
Add additional customization hooks - fixes #61
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