OpenLP

Merge lp:~j-corwin/openlp/migration into lp:openlp

  1. migration
  2. Merge into trunk
Proposed by Raoul Snyman
Status: Merged
Approved by: Raoul Snyman
Approved revision: no longer in the revision history of the source branch.
Merged at revision: not available
Proposed branch: lp:~j-corwin/openlp/migration
Merge into: lp:openlp
Diff against target: None lines
To merge this branch: bzr merge lp:~j-corwin/openlp/migration
Reviewer Review Type Date Requested Status
Tim Bentley Approve
Raoul Snyman approve Approve
Review via email: mp+6131@code.launchpad.net

This proposal supersedes a proposal from 2009-05-02.

To post a comment you must log in.
Revision history for this message
Jonathan Corwin (j-corwin) wrote : Posted in a previous version of this proposal

Attempt to fix song migration so it matches current file definitions.
Remove the need to edit the dmp file as part of the migration

Revision history for this message
Raoul Snyman (raoul-snyman) wrote : Posted in a previous version of this proposal

Muhahaha! The Style Police has arrived!

(Yes, this is mostly old code, but it needs to be said!)

All strings are single-quoted and unicode:

  self.display.output("Songs processing started")

  should be:

  self.display.output(u'Songs processing started')

Why are you adding columns one at a time?

  conn.execute("""create table authors (id integer primary key ASC AUTOINCREMENT);""")
  conn.commit()
  self.display.sub_output("authors table created")
  conn.execute("""alter table authors add column first_name varchar(128);""")
  conn.commit()
  self.display.sub_output("first_name added")
  conn.execute("""alter table authors add column last_name varchar(128);""")
  conn.commit()

  Shouldn't that rather be:

  conn.execute("""
    create table authors (
      id integer primary key ASC AUTOINCREMENT,
      first_name varchar(128),
      last_name varchar(128),
      display_name varchar(255)
    );""")
  conn.commit()
  self.display.sub_output("authors table created")

Also name variables better:

  # I'm not sure what these two variables are supposed to be
  def run_cmd(self, cmd):
      f_i, f_o = os.popen4(cmd)

review: Needs Fixing
Revision history for this message
Jonathan Corwin (j-corwin) wrote : Posted in a previous version of this proposal

On Sat, May 2, 2009 at 9:31 PM, Raoul Snyman <
<email address hidden>> wrote:

> Review: Needs Fixing
> Muhahaha! The Style Police has arrived!
>
> (Yes, this is mostly old code, but it needs to be said!)
>
> All strings are single-quoted and unicode:
>
> self.display.output("Songs processing started")
>
> should be:
>
> self.display.output(u'Songs processing started')

Was already there, nowt to do with me

>
> Why are you adding columns one at a time?
>
> conn.execute("""create table authors (id integer primary key ASC
> AUTOINCREMENT);""")
> conn.commit()
> self.display.sub_output("authors table created")
> conn.execute("""alter table authors add column first_name
> varchar(128);""")
> conn.commit()
> self.display.sub_output("first_name added")
> conn.execute("""alter table authors add column last_name varchar(128);""")
> conn.commit()
>
> Shouldn't that rather be:

> conn.execute("""
> create table authors (
> id integer primary key ASC AUTOINCREMENT,
> first_name varchar(128),
> last_name varchar(128),
> display_name varchar(255)
> );""")
> conn.commit()
> self.display.sub_output("authors table created")

I agree, but again was just copying what was already there since I didn't
know if the original author had done it that way on purpose

>
> Also name variables better:
>
> # I'm not sure what these two variables are supposed to be
> def run_cmd(self, cmd):
> f_i, f_o = os.popen4(cmd)

I didn't touch this function and have no idea what it does.

All I was doing was updating existing code so it worked... I wasn't planning
on rewriting the whole existing
module.

Revision history for this message
Jonathan Corwin (j-corwin) wrote : Posted in a previous version of this proposal

> Muhahaha! The Style Police has arrived!

Changes pushed.

Revision history for this message
Raoul Snyman (raoul-snyman) wrote :

Yes, I know it's not your code, but I'm just saying this for the record...

Use the os.path module functions, they tend to take care of little things:

  conn = sqlite3.connect(self.data_path + os.sep + database)

  should rather be:

  conn = sqlite3.connect(os.path.join(self.data_path, database))

Indices really should have functional names... "author1" is not a good name:

  conn.execute("""create index if not exists author1 on authors (display_name ASC,id ASC);""")

  better:

  conn.execute("""create index if not exists author_display_name on authors (display_name ASC,id ASC);""")

I'm a little wary of all these "replace"s - there's probably a better way...

  replace(replace(replace(replace(replace(replace(replace(replace(

review: Approve (approve)
Revision history for this message
Tim Bentley (trb143) wrote :

Not user about replace(replace(replace as it's done is SQL then thats life

Does this handle Unicode character sets. If it does it simplifes the database migration process.

review: Approve
Revision history for this message
Jonathan Corwin (j-corwin) wrote :

On Sun, May 3, 2009 at 12:37 AM, Raoul Snyman <
<email address hidden>> wrote:
y.

>
> I'm a little wary of all these "replace"s - there's probably a better
> way...
>
> replace(replace(replace(replace(replace(replace(replace(replace(
>
> In the original code it was done in the python as lots of individual
replaces. However the python process of selecting 3000 records (in my case)
and updating each one individually took over 5 minutes on my machine.
Doing it in sqlite direct took 5 seconds...

I agree it doesn't look pretty though.

Jonathan.

lp:~j-corwin/openlp/migration updated
439. By Raoul Snyman

Added images

440. By Raoul Snyman

Merged Jonathan Corwin's changes to the database migration script.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'openlp/migration/migratesongs.py'
--- openlp/migration/migratesongs.py 2009-02-04 20:16:56 +0000
+++ openlp/migration/migratesongs.py 2009-05-02 21:40:10 +0000
@@ -31,231 +31,210 @@
31clear_mappers()31clear_mappers()
32mapper(Author, authors_table)32mapper(Author, authors_table)
33mapper(Book, song_books_table)33mapper(Book, song_books_table)
34mapper(Song, songs_table,34mapper(Song, songs_table, properties={
35 properties={'authors': relation(Author, backref='songs',35 'authors': relation(Author, backref='songs',
36 secondary=authors_songs_table),36 secondary=authors_songs_table),
37 'book': relation(Book, backref='songs'),37 'book': relation(Book, backref='songs'),
38 'topics': relation(Topic, backref='songs',38 'topics': relation(Topic, backref='songs',
39 secondary=songs_topics_table)})39 secondary=songs_topics_table)})
40mapper(Topic, topics_table)40mapper(Topic, topics_table)
4141
42class MigrateSongs():42class MigrateSongs():
43 def __init__(self, display):43 def __init__(self, display):
44 self.display = display44 self.display = display
45 self.config = PluginConfig("Songs")45 self.config = PluginConfig('Songs')
46 self.data_path = self.config.get_data_path()46 self.data_path = self.config.get_data_path()
47 self.database_files = self.config.get_files("sqlite")47 self.database_files = self.config.get_files('sqlite')
48 print self.database_files48 print self.database_files
4949
50 def process(self):50 def process(self):
51 self.display.output("Songs processing started");51 self.display.output(u'Songs processing started')
52 for f in self.database_files:52 for f in self.database_files:
53 self.v_1_9_0(f)53 self.v_1_9_0(f)
54 self.display.output("Songs processing finished");54 self.display.output(u'Songs processing finished')
5555
56 def v_1_9_0(self, database):56 def v_1_9_0(self, database):
57 self.display.output("Migration 1.9.0 Started for "+database);57 self.display.output(u'Migration 1.9.0 Started for ' + database)
58 self._v1_9_0_authors(database)58 self._v1_9_0_authors(database)
59 self._v1_9_0_topics(database)59 self._v1_9_0_topics(database)
60 self._v1_9_0_songbook(database)60 self._v1_9_0_songbook(database)
61 self._v1_9_0_songauthors(database)61 self._v1_9_0_songauthors(database)
62 self._v1_9_0_songtopics(database)62 self._v1_9_0_songtopics(database)
63 self._v1_9_0_songs(database)63 self._v1_9_0_songs(database)
64 self._v1_9_0_songs_update(database)64 self.display.output(u'Migration 1.9.0 Finished for ' + database)
65
66 self.display.output("Migration 1.9.0 Finished for " + database);
6765
68 def _v1_9_0_authors(self, database):66 def _v1_9_0_authors(self, database):
69 self.display.sub_output("Authors Started for "+database);67 self.display.sub_output(u'Authors Started for ' + database)
70 conn = sqlite3.connect(self.data_path+os.sep+database)68 conn = sqlite3.connect(self.data_path + os.sep + database)
71 conn.execute("""alter table authors rename to authors_temp;""")69 conn.execute("""alter table authors rename to authors_temp;""")
72 conn.commit()70 conn.commit()
7371 self.display.sub_output(u'old author renamed to author_temp')
74 conn.execute("""create table authors add column display_name varchar(255);""")72 conn.execute("""create table authors (
75 conn.commit()73 id integer primary key ASC AUTOINCREMENT,
76 self.display.sub_output("first name created")74 first_name varchar(128),
77 conn.execute("""alter table authors add column first_name varchar(128);""")75 last_name varchar(128),
78 conn.commit()76 display_name varchar(255)
79 self.display.sub_output("first name created")77 );""")
80 conn.execute("""alter table authors add column last_name varchar(128);""")78 conn.commit()
81 conn.commit()79 self.display.sub_output(u'authors table created')
82 self.display.sub_output("last name created")80 conn.execute("""create index if not exists author1 on authors
83 conn.execute("""create index if not exists author_display_name on authors (display_name ASC,id ASC);""")81 (display_name ASC,id ASC);""")
84 conn.commit()82 conn.commit()
85 self.display.sub_output("index author1 created")83 self.display.sub_output(u'index author1 created')
86 conn.execute("""create index if not exists author_last_name on authors (last_name ASC,id ASC);""")84 conn.execute("""create index if not exists author2 on authors
87 conn.commit()85 (last_name ASC,id ASC);""")
88 self.display.sub_output("index author2 created")86 conn.commit()
89 conn.execute("""create index if not exists author_first_name on authors (first_name ASC,id ASC);""")87 self.display.sub_output(u'index author2 created')
90 conn.commit()88 conn.execute("""create index if not exists author3 on authors
91 self.display.sub_output("index author3 created")89 (first_name ASC,id ASC);""")
92 self.display.sub_output("Author Data Migration started")90 conn.commit()
93 c = conn.cursor()91 self.display.sub_output(u'index author3 created')
94 text = c.execute("""select * from authors """) .fetchall()92 self.display.sub_output(u'Author Data Migration started')
93 conn.execute("""insert into authors (id, display_name)
94 select authorid, authorname from authors_temp;""")
95 conn.commit()
96 self.display.sub_output(u'authors populated')
97 c = conn.cursor()
98 text = c.execute("""select * from authors""") .fetchall()
95 for author in text:99 for author in text:
96 dispname = author[1]100 dispname = author[3]
97 if author[2] == None:101 dispname = dispname.replace("'", "")
98 dispname = dispname.replace("'", "") # remove quotes.102 pos = dispname.rfind(" ")
99 pos = dispname.rfind(" ")103 authorfirstname = dispname[:pos]
100 afn = dispname[:pos]104 authorlastname = dispname[pos + 1:len(dispname)]
101 aln = dispname[pos + 1:len(dispname)]105 s = "update authors set first_name = '" \
102 #txt = text[2]106 + authorfirstname + "', last_name = '" + authorlastname \
103 s = "update authors set display_name = '" + dispname + "', first_name = '" + afn + "', last_name = '" + aln + "' where id = " +str(author[0])107 + "' where id = " + str(author[0])
104 text1 = c.execute(s)108 c.execute(s)
105 conn.commit()109 conn.commit()
106 conn.execute("""alter table authors drop column authorname;""")110 self.display.sub_output(u'Author Data Migration Completed')
107 conn.commit()111 conn.execute("""drop table authors_temp;""")
108 conn.close()112 conn.commit()
109 self.display.sub_output("Author Data Migration Completed")113 conn.close()
110 self.display.sub_output("Authors Completed");114 self.display.sub_output(u'author_temp dropped')
115 self.display.sub_output(u'Authors Completed')
116
117 def _v1_9_0_songbook(self, database):
118 self.display.sub_output(u'SongBook Started for ' + database)
119 conn = sqlite3.connect(self.data_path + os.sep + database)
120 conn.execute("""create table if not exists song_books (
121 id integer Primary Key ASC AUTOINCREMENT,
122 name varchar(128),
123 publisher varchar(128)
124 );""")
125 conn.commit()
126 self.display.sub_output(u'songbook table created')
127 conn.execute("""create index if not exists songbook1 on song_books (name ASC,id ASC);""")
128 conn.commit()
129 self.display.sub_output(u'index songbook1 created')
130 conn.execute("""create index if not exists songbook2 on song_books (publisher ASC,id ASC);""")
131 conn.commit()
132 conn.close()
133 self.display.sub_output(u'index songbook2 created')
134 self.display.sub_output(u'SongBook Completed')
135
136 def _v1_9_0_songs(self, database):
137 self.display.sub_output(u'Songs Started for ' + database)
138 conn = sqlite3.connect(self.data_path + os.sep + database)
139 conn.execute("""alter table songs rename to songs_temp;""")
140 conn.commit()
141 conn.execute("""create table if not exists songs (
142 id integer Primary Key ASC AUTOINCREMENT,
143 song_book_id integer,
144 title varchar(255),
145 lyrics text,
146 verse_order varchar(128),
147 copyright varchar(255),
148 comments text,
149 ccli_number varchar(64),
150 song_number varchar(64),
151 theme_name varchar(128),
152 search_title varchar(255),
153 search_lyrics text
154 );""")
155 conn.commit()
156 self.display.sub_output(u'songs table created')
157 conn.execute("""create index if not exists songs1 on songs
158 (search_lyrics ASC,id ASC);""")
159 conn.commit()
160 self.display.sub_output(u'index songs1 created')
161 conn.execute("""create index if not exists songs2 on songs
162 (search_title ASC,id ASC);""")
163 conn.commit()
164 self.display.sub_output(u'index songs2 created')
165 conn.execute("""insert into songs (id, title, lyrics, copyright,
166 search_title, search_lyrics, song_book_id)
167 select songid, songtitle, lyrics, copyrightinfo,
168 replace(replace(replace(replace(replace(replace(replace(replace(
169 replace(songtitle, '&', 'and'), ',', ''), ';', ''), ':', ''),
170 '(', ''), ')', ''), '{', ''), '}',''),'?',''),
171 replace(replace(replace(replace(replace(replace(replace(replace(
172 replace(lyrics, '&', 'and'), ',', ''), ';', ''), ':', ''),
173 '(', ''), ')', ''), '{', ''), '}',''),'?',''),
174 0
175 from songs_temp;""")
176 conn.commit()
177 self.display.sub_output(u'songs populated')
178 conn.execute("""drop table songs_temp;""")
179 conn.commit()
180 conn.close()
181 self.display.sub_output(u'songs_temp dropped')
182 self.display.sub_output(u'Songs Completed')
111183
112 def _v1_9_0_topics(self, database):184 def _v1_9_0_topics(self, database):
113 self.display.sub_output("Topics Started for "+database);185 self.display.sub_output(u'Topics Started for ' + database)
114 conn = sqlite3.connect(self.data_path+os.sep+database)186 conn = sqlite3.connect(self.data_path+os.sep+database)
115 conn.text_factory = str187 conn.text_factory = str
116 conn.execute("""create table if not exists topics (id integer Primary Key ASC AUTOINCREMENT);""")188 conn.execute("""create table if not exists topics
117 conn.commit()189 (id integer Primary Key ASC AUTOINCREMENT,
118 self.display.sub_output("Topic table created")190 name varchar(128));""")
119 conn.execute("""alter table topics add column name varchar(128);""")191 conn.commit()
120 conn.commit()192 self.display.sub_output(u'Topic table created')
121 self.display.sub_output("topicname added")
122 conn.execute("""create index if not exists topic1 on topics (name ASC,id ASC);""")193 conn.execute("""create index if not exists topic1 on topics (name ASC,id ASC);""")
123 conn.commit()194 conn.commit()
124 conn.close()195 conn.close()
125 self.display.sub_output("index topic1 created")196 self.display.sub_output(u'index topic1 created')
126197
127 self.display.sub_output("Topics Completed");198 self.display.sub_output(u'Topics Completed')
128199
129 def _v1_9_0_songbook(self, database):200 def _v1_9_0_songauthors(self, database):
130 self.display.sub_output("SongBook Started for "+database);201 self.display.sub_output(u'SongAuthors Started for ' + database);
131 conn = sqlite3.connect(self.data_path+os.sep+database)202 conn = sqlite3.connect(self.data_path + os.sep + database)
132 conn.execute("""create table if not exists song_books (id integer Primary Key ASC AUTOINCREMENT);""")203 conn.execute("""create table if not exists authors_songs
133 conn.commit()204 (author_id integer,
134 self.display.sub_output("SongBook table created")205 song_id integer);""")
135 conn.execute("""alter table song_books add column name varchar(128);""")206 conn.commit()
136 conn.commit()207 self.display.sub_output(u'authors_songs table created')
137 self.display.sub_output("songbook_name added")208 conn.execute("""insert into authors_songs (author_id, song_id)
138 conn.execute("""alter table song_books add column publisher varchar(128);""")209 select authorid, songid from songauthors;""")
139 conn.commit()210 conn.commit()
140 self.display.sub_output("songbook_publisher added")211 self.display.sub_output(u'authors_songs populated')
141 conn.execute("""create index if not exists songbook1 on song_books (name ASC,id ASC);""")212 conn.execute("""drop table songauthors;""")
142 conn.commit()213 conn.commit()
143 self.display.sub_output("index songbook1 created")214 self.display.sub_output(u'songauthors dropped')
144 conn.execute("""create index if not exists songbook2 on song_books (publisher ASC,id ASC);""")
145 conn.commit()
146 conn.close()215 conn.close()
147 self.display.sub_output("index songbook2 created")216 self.display.sub_output(u'SongAuthors Completed')
148 self.display.sub_output("SongBook Completed");
149217
150 def _v1_9_0_songtopics(self, database):218 def _v1_9_0_songtopics(self, database):
151 self.display.sub_output("Songtopics Started for "+database);219 self.display.sub_output(u'Songtopics Started for ' + database);
152 conn = sqlite3.connect(self.data_path+os.sep+database)220 conn = sqlite3.connect(self.data_path+os.sep+database)
153 conn.execute("""create table if not exists song_topics (song_id integer Primary Key ASC );""")221 conn.execute("""create table if not exists song_topics
154 conn.commit()222 (song_id integer,
155 self.display.sub_output("Songtopics table created")223 topic_id integer);""")
156 conn.execute("""alter table song_topics add column topic_id interger;""")224 conn.commit()
157 conn.commit()225 self.display.sub_output(u'songtopics table created')
158 self.display.sub_output("songtopics_topic_id added")
159 conn.execute("""create index if not exists songtopic1 on song_topics (topic_id ASC,song_id ASC);""")226 conn.execute("""create index if not exists songtopic1 on song_topics (topic_id ASC,song_id ASC);""")
160 conn.commit()227 conn.commit()
161 self.display.sub_output("index songbook1 created")228 self.display.sub_output(u'index songtopic1 created')
162 conn.execute("""create index if not exists songbook2 on song_topics (song_id ASC,topic_id ASC);""")229 conn.execute("""create index if not exists songtopic2 on song_topics (song_id ASC,topic_id ASC);""")
163 conn.commit()230 conn.commit()
164 conn.close()231 conn.close()
165 self.display.sub_output("index songbook2 created")232 self.display.sub_output(u'index songtopic2 created')
166 self.display.sub_output("SongTopics Completed");233 self.display.sub_output(u'SongTopics Completed')
167
168 def _v1_9_0_songauthors(self, database):
169 self.display.sub_output("SongAuthors Started for "+database);
170 conn = sqlite3.connect(self.data_path+os.sep+database)
171 conn.execute("""alter table songauthors rename to authors_songs;""")
172 conn.commit()
173 conn.close()
174 self.display.sub_output("Table Renamed")
175 self.display.sub_output("SongAuthors Completed");
176
177
178 def _v1_9_0_songs(self, database):
179 self.display.sub_output("Songs Started for "+database);
180 conn = sqlite3.connect(self.data_path+os.sep+database)
181 conn.execute("""alter table songs add column song_book_id interger;""")
182 conn.commit()
183 self.display.sub_output("songs_song_book_id added")
184 conn.execute("""alter table songs add column verse_order varchar(128);""")
185 conn.commit()
186 self.display.sub_output("songs verse_order added")
187 conn.execute("""alter table songs add column comments text;""")
188 conn.commit()
189 self.display.sub_output("songs comments added")
190 conn.execute("""alter table songs add ccli_number varchar(64);""")
191 conn.commit()
192 self.display.sub_output("songs ccli_number added")
193 conn.execute("""alter table songs add song_number varchar(64);""")
194 conn.commit()
195 self.display.sub_output("songs song_number added")
196 conn.execute("""alter table songs add theme_name varchar(128);""")
197 conn.commit()
198 self.display.sub_output("songs theme_name added")
199 conn.execute("""alter table songs add search_title varchar(255);""")
200 conn.commit()
201 self.display.sub_output("songs search_title added")
202 conn.execute("""alter table songs add search_lyrics text;""")
203 conn.commit()
204 self.display.sub_output("songs search_lyrics added")
205
206 conn.execute("""create index if not exists songs1 on songs (search_lyrics ASC,id ASC);""")
207 conn.commit()
208 self.display.sub_output("index songs1 created")
209 conn.execute("""create index if not exists songs2 on songs (search_title ASC,id ASC);""")
210 conn.commit()
211 conn.close()
212 self.display.sub_output("index songs2 created")
213 self.display.sub_output("Songs Completed");
214
215
216
217 def _v1_9_0_songs_update(self, database):
218 self.display.sub_output("Songs Started for "+database);
219 self.db = create_engine("sqlite:///"+self.data_path+os.sep+database, encoding='utf-8' , convert_unicode=False, assert_unicode=False)
220
221 self.db.echo = True
222 metadata.bind = self.db
223 metadata.bind.echo = False
224 Session = scoped_session(sessionmaker(autoflush=True, autocommit=False))
225 Session.configure(bind=self.db)
226 #create missing table
227 songs_topics_table.create()
228 songs = Session.query(Song).all()
229 for song in songs:
230 t=song.title.replace("&", "and")
231 t=t.replace("'", "")
232 t=t.replace(",", "")
233 t=t.replace(";", "")
234 t=t.replace(":", "")
235 t=t.replace("(", "")
236 t=t.replace(")", "")
237 t=t.replace("{", "")
238 t=t.replace("}", "")
239 t=t.replace("?", "")
240 song.search_title = t
241 t=song.lyrics.replace("&", "and")
242 t=t.replace("'", "")
243 t=t.replace(",", "")
244 t=t.replace(";", "")
245 t=t.replace(":", "")
246 t=t.replace("(", "")
247 t=t.replace(")", "")
248 t=t.replace("{", "")
249 t=t.replace("}", "")
250 t=t.replace("?", "")
251 song.search_lyrics = t
252 song.song_book_id = 0
253 Session.save_or_update(song)
254 Session.commit()
255234
256 def run_cmd(self, cmd):235 def run_cmd(self, cmd):
257 f_i, f_o = os.popen4(cmd)236 filein, fileout = os.popen4(cmd)
258 out = f_o.readlines()237 out = fileout.readlines()
259 if len(out) > 0:238 if len(out) > 0:
260 for o in range (0, len(out)):239 for o in range (0, len(out)):
261 self.display.sub_output(out[o])240 self.display.sub_output(out[o])
262241
=== modified file 'openlpcnv.pyw' (properties changed: -x to +x)