Code

Opened 7 years ago

Closed 6 years ago

#4747 closed (wontfix)

[multi-db] patch to bring multiple-db-support up to date with rev 6110

Reported by: ben <ben.fordnz@…> Owned by: nobody
Component: Uncategorized Version: other branch
Severity: Keywords: multiple-db-support
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: UI/UX:

Description

These are my updates to the multiple-db-support branch, bringing it up to date with trunk (-r5559). I've attached incremental patches from -r4189 which is when it was last worked on and also on comprehensive patch against trunk. There are still some tests failing, and I'd like for people to be able to get eyes on the code to try and give work on the branch a bump start!

Attachments (8)

mdb.patch (203.1 KB) - added by ben <ben.fordnz@…> 7 years ago.
Full Patch -r4189:5589
multi-db-6110.diff (188.6 KB) - added by ben <ben.fordnz@…> 7 years ago.
Koens diff against trunk -r6110.
multi-db-6110.patch (188.6 KB) - added by ben <ben.fordnz@…> 7 years ago.
Koen patch against trunk -r6110. Renamed extension to patch so you can see it in the html interface.
multidb_6433.diff (116.0 KB) - added by Koen Biermans <koen.biermans@…> 7 years ago.
patch for trunk r6433 (beware for management commands)
multidb_6453.diff (118.8 KB) - added by Koen Biermans <koen.biermans@…> 7 years ago.
patch that applies ok to r6453 (I screwed up the previous one)
mymultidb_trunk7534_20080520.diff (143.1 KB) - added by Koen Biermans <koen.biermans@…> 6 years ago.
effort to get multidb into trunk r7534
mymultidb_trunk7534_20080520.2.diff (139.8 KB) - added by Koen Biermans <koen.biermans@…> 6 years ago.
effort to get multidb into trunk r7534
mymultidb_trunk7534_20080521.diff (137.3 KB) - added by Koen Biermans <koen.biermans@…> 6 years ago.
new attempt to generate correct diff

Download all attachments as: .zip

Change History (24)

Changed 7 years ago by ben <ben.fordnz@…>

Full Patch -r4189:5589

comment:1 Changed 7 years ago by ben <ben.fordnz@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Sorry, I couldn't add the incremental patches for some reason... This is the full patch to bring multiple-db-support up to date with rev 5598

comment:2 Changed 7 years ago by Simon G. <dev@…>

  • Summary changed from Updates to multiple-db-support to [multi-db] patch to bring multiple-db-support up to date with rev 5598
  • Triage Stage changed from Unreviewed to Ready for checkin
  • Version changed from SVN to other branch

comment:3 Changed 7 years ago by (removed)

@ben, where are you mirroring this? Specifically, got a vcs branch for this?

Changed 7 years ago by ben <ben.fordnz@…>

Koens diff against trunk -r6110.

Changed 7 years ago by ben <ben.fordnz@…>

Koen patch against trunk -r6110. Renamed extension to patch so you can see it in the html interface.

comment:4 Changed 7 years ago by ben <ben.fordnz@…>

Koen has supplied a patch bringing multi-db up to -r6110. This is a cut-n-past of the email he sent to me regarding the changes he's made:

  • removed (most of) the schema evolution stuff (also the tests)
  • get_db_prep_save gets the model instance so it can determine what backend it uses (needed for mysql exception in datetimefield
  • queryset gets set in manager since some backends have custom querysets (eg oracle)
  • (hopefully) all quote_name references have been resolved to the models connection ops via model._default_manager.db.connection.ops.quote_name
  • query.py: quote_only_if_word uses the backends quote_name (passed in)
  • management stuff: syncdb put model connection determination all around, seems to work, needs testing
  • validation: needs a closer look
  • sql: table_list ok, create ok, delete ?, flush NOT OK, others TO DO
  • thread_isolation test failing : TODO
  • test_client test failing (flush?): TODO

This means Unicode, backend and management refactoring should be more or less ok.

There are some things to look at, but I need a break now.

I have not done to much testing yet though. I’ll see when I can get round to that (maybe later this evening when the kids are asleep).

I hope this helps.

Koen

comment:5 Changed 7 years ago by micsco

  • Needs documentation set
  • Needs tests set
  • Summary changed from [multi-db] patch to bring multiple-db-support up to date with rev 5598 to [multi-db] patch to bring multiple-db-support up to date with rev 6110
  • Triage Stage changed from Ready for checkin to Accepted

comment:6 Changed 7 years ago by levity@…

it doesn't work for me. i get:

Traceback (most recent call last):
  File "/home/cdn/ui/scripts/send_alerts.py", line 1, in <module>
    from tools.alert import email_live_alerts
  File "/home/cdn/ui/scripts/tools/__init__.py", line 12, in <module>
    from django.db import connection
  File "/usr/local/lib/python2.5/site-packages/django/db/__init__.py", line 356, in <module>
    IntegrityError = backend.IntegrityError
  File "/usr/local/lib/python2.5/site-packages/django/db/__init__.py", line 325, in __getattr__
    **self.__kw))
  File "/usr/local/lib/python2.5/site-packages/django/db/__init__.py", line 352, in <lambda>
    lambda: connections[_default].backend)
  File "/usr/local/lib/python2.5/site-packages/django/db/__init__.py", line 147, in __getitem__
    return self.connect(k)
  File "/usr/local/lib/python2.5/site-packages/django/db/__init__.py", line 170, in connect
    cnx[name] = connect(settings)
  File "/usr/local/lib/python2.5/site-packages/django/db/__init__.py", line 38, in connect
    return ConnectionInfo(settings, **kw)
  File "/usr/local/lib/python2.5/site-packages/django/db/__init__.py", line 55, in __init__
    self.connection = self.backend.DatabaseWrapper(settings)
TypeError: __init__() takes exactly 1 argument (2 given)

i'm using two mysql backends.

comment:7 Changed 7 years ago by ben <ben.fordnz@…>

Without some more detail this might be a bit difficult to diagnose... I have noticed that mysql_old DatabaseWrapper's init looks like:

def init(self, kwargs)

Although that still doesn't explain why it chokes on the settings argument... Can you provide more detail:

  • What version of trunk
  • Which patch
  • A diff of backends/mysql/base.py (or the whole file)
  • If you could have a play inside ipython using pdb that would probably throw up some interesting info too!

Cheers

comment:8 Changed 7 years ago by anonymous

i'm using mysql, not mysql_old... but it also has __init__(self, **kwargs).

but the call to it from db/__init__.py is DatabaseWrapper(settings), without a keyword specified, so it worked when i changed the constructor to __init__(self, *args, **kwargs). but then i ran into another error, so i thought maybe i was doing something more fundamentally wrong with the patch.

all i did was pull a fresh copy of r6110, and apply multi-db-6110.patch.

comment:9 Changed 7 years ago by ben <ben.fordnz@…>

Ok,
I just had a quick look through that patch and it doesn't seem to have changed any of the base.py files in respective backends. I'm not sure why this is, I know Koen looked at a lot of things and tried to refactor a bit to take advantage of recent changes in trunk. All I can say is that it looks pretty different from my earlier patch, and I'm not sure why! I wish I had time to look into this for you today, but I really can't, I have way too much on. All I can suggest is sending an email to Koen and ask him to help out (He's in Belgium so it'll be a few more hours until he's up!)
Sorry I can't be of more help!
Ben

Changed 7 years ago by Koen Biermans <koen.biermans@…>

patch for trunk r6433 (beware for management commands)

comment:10 Changed 7 years ago by Koen Biermans <koen.biermans@…>

  • Patch needs improvement set

Ok, the patch is now up with 6433. Beware! This was and is work in progress. Especially the management.py commands are still in a bad state. A number of the commands should be made to accept a parameter for the named database connection (which is not the case now). There are also other areas where I think things need to be redone. For me now a lot seems to work (using existing databases that is).
Sorry that I haven't had much time to proceed on this any further.
Koen

Changed 7 years ago by Koen Biermans <koen.biermans@…>

patch that applies ok to r6453 (I screwed up the previous one)

comment:11 Changed 7 years ago by mail@…

according to the patch multidb_6453.diff:

there's an error with filefields. In django/db/models/fields/init.py the method get_db_prep_save for class FileField has to be changed to this:

    def get_db_prep_save(self, model_instance, value):
        "Returns field's value prepared for saving into a database."
        # Need to convert UploadedFile objects provided via a form to unicode for database insertion
        if value is None:
            return None
        return Field.get_db_prep_save(self, model_instance, unicode(value))

Changed 6 years ago by Koen Biermans <koen.biermans@…>

effort to get multidb into trunk r7534

comment:12 Changed 6 years ago by Koen Biermans <koen.biermans@…>

A new effort to bring multiple database support into trunk.

The patch is from trunk r7534. A lot seems to be working, but there are still some areas that need looking into.
I corrected some of the management commands (the sql ones), but eg loaddata and sequencereset are still to be looked at.
It is passing most of the tests (with sqlite that is), except the thread-isolation that came from the multidb branch.
I have only tried it with sqlite and postgres, so for instance oracle will probably not work (since I am not yet passing separate query classes in for those backends that have a custom query class).

I'm continuing the work, but please feel free to help out!

Changed 6 years ago by Koen Biermans <koen.biermans@…>

effort to get multidb into trunk r7534

comment:13 Changed 6 years ago by mail@…

I cannot get the patch working. When trying to apply it, it always reports failures:

# patch -p0 --verbose --dry-run < ./multidb.diff
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|=== django/test/simple.py
|==================================================================
|--- django/test/simple.py      (/mirror/django/trunk)  (revision 5420)
|
|+++ django/test/simple.py      (/local/django/mymultidb)       (revision 5420)
|
--------------------------
Patching file django/test/simple.py using Plan A...
Hunk #1 succeeded at 137 with fuzz 2 (offset -1 lines).
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|
|=== django/test/utils.py
|==================================================================
|--- django/test/utils.py       (/mirror/django/trunk)  (revision 5420)
|
|+++ django/test/utils.py       (/local/django/mymultidb)       (revision 5420)
|
--------------------------
Patching file django/test/utils.py using Plan A...
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file django/test/utils.py.rej
Hmm...missing header for unified diff at line 39 of patch
  The next patch looks like a unified diff to me...
can't find file to patch at input line 39
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
| from django.dispatch import dispatcher
|
--------------------------
File to patch:

(and lots more... even at django/models/sql/where.py)

it seems, that only the first part per file of the diff gets applied. What's wrong here?

Changed 6 years ago by Koen Biermans <koen.biermans@…>

new attempt to generate correct diff

comment:14 Changed 6 years ago by Koen Biermans <koen.biermans@…>

Sorry about that, I created the diff with svk (on windows), which obviously is not working right.

I created one with svn now, I hope this one is allright.

comment:15 Changed 6 years ago by Koen Biermans <koen.biermans@…>

Ben Ford set up a mercurial repository for new work on multiple databases at http://hg.woe-beti.de. A track setup is available at http://trac.woe-beti.de.

comment:16 Changed 6 years ago by mtredinnick

  • Resolution set to wontfix
  • Status changed from new to closed

The multi-db is no longer active. Alternative approaches are being considered for trunk.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.