cloudbees-pull-request-builder
commented
10 years ago jsword #68 FAILURE Looks like there's a problem with this pull request
Closed sijocherian closed 10 years ago
cloudbees-pull-request-builder
commented
10 years ago jsword #68 FAILURE Looks like there's a problem with this pull request
dmsmith
commented
10 years ago I think that we don't need to wait for answers on the open questions to merge this. I'll reply on jsword-devel to them.
mjdenham
commented
10 years ago "How do the clients want the DownloadIndex option to work with index versioning?"
I believe And Bible is the only JSword app that uses the DownloadIndex feature at the moment. You will find a brief description if you look for "Make AbstractSwordInstaller.downloadSearchIndex configurable" in jsword-devel or "Location of prebuilt lucene indexes" in sword-devel.
The downloadSearchIndex method is not structured the way AB needs it to be so there is an AB index downloader which adopts a similar approach to PocketSword. Code is here. It would be good to make this more generic and push it back into JSword if it is useful.
The indexes downloaded can be seen here. As you can see the url contains a version number which is currently 'v1'. We could make this match the index version number (1.2) you have been referring to.
Regarding the higher level design it might be good to have a toggle in reindexAllBooksIfNeeded so that the indexes could be downloaded if available because creating indexes on small mobiles can take a long time. However, even downloading can take a long time so no reindex should be mandatory.
AB doesn't automatically install indexes atm so many users will only have some books indexed. Have you decided what reindexAllBooksIfNeeded should do for unindexed books?
Martin
dmsmith
commented
10 years ago I know of no other JSword app, other than And Bible, that uses downloadable indexes. It turned out to be very complicated to support multiple versions of an index for a module. Maybe someday we'll get there.
This will be a step in the right direction.
I'll add that it is better to treat a module with an invalid index as one that has no index with regard to search. I've gotten really goofy results when the index is no longer valid. In Bible Desktop, I plan to have the Enable Search button show up if the index does not exist or is invalid. When clicked, an invalid index will be deleted and then created. Maybe the label should be different for an invalid index, e.g. "Re-enable Search".
DM
On Apr 5, 2014, at 1:30 PM, Martin notifications@github.com wrote:
"How do the clients want the DownloadIndex option to work with index versioning?"
I believe And Bible is the only JSword app that uses the DownloadIndex feature at the moment. You will find a brief description if you look for "Make AbstractSwordInstaller.downloadSearchIndex configurable" in jsword-devel or "Location of prebuilt lucene indexes" in sword-devel.
The downloadSearchIndex method is not structured the way AB needs it to be so there is an AB index downloader which adopts a similar approach to PocketSword. Code is here. It would be good to make this more generic and push it back into JSword if it is useful.
The indexes downloaded can be seen here. As you can see the url contains a version number which is currently 'v1'. We could make this match the index version number (1.2) you have been referring to.
Regarding the higher level design it might be good to have a toggle in reindexAllBooksIfNeeded so that the indexes could be downloaded if available because creating indexes on small mobiles can take a long time. However, even downloading can take a long time so no reindex should be mandatory.
AB doesn't automatically install indexes atm so many users will only have some books indexed. Have you decided what reindexAllBooksIfNeeded should do for unindexed books?
Martin
— Reply to this email directly or view it on GitHub.
sijocherian
commented
10 years ago Thanks Martin,DM.
Based on comments reindexAllBooksIfNeeded() can do this:
for each Books.installed() { if(book's index exist AND needsReindexing) reindex & update Books Installed.Index.Version } set Installed.Index.DefaultVersion={Latest Version}
For now: thinking separate method like downloadLatestIndexForAllBooksIfNeeded(param baseURL etc)
sijo
dmsmith
commented
10 years ago Sijo,
I'm probably atypical, but I have over 500 books installed. Most are Bibles. If I updated the program and that invalidated every index, would it be a good idea for the program to download all new indexes or to generate all of them one after the other? What happens if I kill the program in the process? Should there be a way to say later?
BTW, my laptop is fast enough to take the hit of doing that kind of effort. I have an SSD and a fast processor.
It just dawned on me that the module's version number needs to be part of the re-index rule (you may have already thought of this).
DM
On Apr 6, 2014, at 7:50 PM, Sijo Cherian notifications@github.com wrote:
Thanks Martin,DM.
Based on comments reindexAllBooksIfNeeded() can do this:
for each Books.installed() { if(book's index exist AND needsReindexing) reindex & update Books Installed.Index.Version } set Installed.Index.DefaultVersion={Latest Version}
For now: thinking separate method like downloadLatestIndexForAllBooksIfNeeded(param baseURL etc)
sijo
— Reply to this email directly or view it on GitHub.
Following is the upgrade criteria implemented by comparing IndexMetadata from prop files: needsReindexing returns true, if Latest.Index.Version.xxx > Installed.Index.Version.xxx OR if {index folder} is not found at all.
Notes: Looked into storing version values in the lucene index using FlexibleIndexing in lucene 4.0, but that seem very dependent on lucene api. Prop file gives human readable configuration and easy to debug.
ToDo/Questions: