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

Added batch size option to the management command for populating the history #231

Closed
wants to merge 1 commit into from

Conversation

mikeengland
Copy link

Currently, if you try and populate the history of a large table Django raises a 'lost connection to MySQL server' error. This is because the bulk_create method is not given a batch size.

This pull request adds an additional parameter to the management command (--batchsize), where you can specify this e.g. 500. I have set the default value to a reasonable 200. Issue #202 and #189 should be fixed in this pull request.

@codecov-io
Copy link

codecov-io commented Aug 1, 2016

Current coverage is 96.85% (diff: 100%)

Merging #231 into master will not change coverage

@@             master       #231   diff @@
==========================================
  Files             9          9          
  Lines           445        445          
  Methods           0          0          
  Messages          0          0          
  Branches         56         56          
==========================================
  Hits            431        431          
  Misses            7          7          
  Partials          7          7          

Powered by Codecov. Last update 6580dde...7753d38

@mikeengland
Copy link
Author

mikeengland commented Aug 1, 2016

Hmm... looks like some of the TravisCI builds have failed due to the error OperationalError: no such table: tests_historicaltrackedwithmultipleabstractbases. Has anyone seen this before? (I'm not too familiar with SQL Lite).

@macro1
Copy link
Collaborator

macro1 commented Aug 1, 2016

Yes, I've seen errors like this before. There are some race conditions that can surface because we're dynamically creating models within some of the tests.

The table giving you trouble here is correctly missing... I'm not sure why Django thinks it should exist though. Here's the code in the tests: https://github.com/treyhunner/django-simple-history/blob/1.8.1/simple_history/tests/tests/test_models.py#L801

Of course these errors don't have anything to do with your change, or shouldn't. I can look at them later.

@mikeengland
Copy link
Author

Ok, thanks @macro1 !

@mikeengland
Copy link
Author

Please can someone take a look at merging this? It would be very useful to have in master.

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

Successfully merging this pull request may close these issues.

3 participants