Propagation of library updates to the embedded kiwix server #714
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The new helper class `KiwixApp::NameMapperProxy` serves as a level of indirection for `kiwix::HumanReadableNameMapper`. It will help to solve the problem of updating the mapping when the library changes.
The chosen synchronization scheme between update() and getNameForId()/getIdForName() optimizes the duration of the critical sections - the getNameForId()/getIdForName() operations are not locked for their full duration of execution.
veloman-yunkan
force-pushed
the
uptodate_name_mapper
branch
from
c3dbf7e to
44ddebf
Compare
mgautierfr
approved these changes
Nov 3, 2021
There was a problem hiding this comment.
The solution should be moved in libkiwix soon but I agree we can go with this "local" solution and move it to libkiwix with the work on kiwix/kiwix-tools#243
|
@veloman-yunkan @mgautierfr I just tested, and the symptom described in my ticket kiwix/libkiwix#629 ist still there. |
@kelson42 Something is wrong with your testing. It works for me. Did you build the code yourself? Are you sure that the fix is included in your build? |
veloman-yunkan
added a commit
to kiwix/libkiwix
that referenced
this pull request
Nov 20, 2021
The right place for NameMapperProxy introduced by kiwix/kiwix-desktop#714 is in libkiwix (so that it can be reused in kiwix-serve).
veloman-yunkan
added a commit
to kiwix/libkiwix
that referenced
this pull request
Nov 21, 2021
veloman-yunkan
added a commit
to kiwix/libkiwix
that referenced
this pull request
Nov 22, 2021
The right place for NameMapperProxy introduced by kiwix/kiwix-desktop#714 is in libkiwix (so that it can be reused in kiwix-serve).
veloman-yunkan
added a commit
to kiwix/libkiwix
that referenced
this pull request
Nov 30, 2021
The right place for NameMapperProxy introduced by kiwix/kiwix-desktop#714 is in libkiwix (so that it can be reused in kiwix-serve).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes kiwix/libkiwix#629
The problem is solved by fully recreating the name mapper whenever the library changes. An alternative solution was to add new methods
addBook()andremoveBook()toHumanReadableNameMapper(or to a new subclass of it).The primary goal was to constrain the fix to a single repo (so that the reported issue is promptly resolved by one PR). However, this or the alternative solution will need to be migrated to libkiwix so that it can be reused for kiwix/kiwix-tools#243.