Just installed 5.2 UndoHistory experimental addon by @DavidMStraub. There wasn’t a README.md to describe the type of plugin nor an addons.json to let the Addon Manager manage the installation yet. So used GitHub’s “Download ZIP” for repositories and the Isotammi ZipInstall to install the addon.
The database engine successfully registers but gives a lot of errors when trying to convert a SQLite tree. (A bug has been filed.)
Addon Manager would have refused to install the plugin until the sqlalchemy prerequisite was available.
But shouldn’t the Plugin Registration fail on restart of Gramps if a prerequisite is missing too?
Since SQLalchemy is a prerequite and the 5.2 Windows AIO includes pip, tried to use pip from the command line.
pip install SQLAlchemy
But PIP was not found in the command path.
PIP.exe is located in C:\Program Files\GrampsAIO64-5.2.0
since Windows doesn’t like spaces, the command line becomes:
C:\Progra~1\GrampsAIO64-5.2.0\pip install SQLAlchemy
The SQLAlchemy requirement is commented out in the registration file. Otherwise Gramps would refuse to load the addon unless the requirement was met.
1 Like
Getting an error using PIP to install:
C:\Progra~1\GrampsAIO64-5.2.0\pip install SQLAlchemy
DEPRECATION: Loading egg at C:\Progra~1\GrampsAIO64-5.2.0\lib\library.zip is deprecated. pip 24.3 will enforce this behaviour change. A possible replacement is to use pip for package installation.. Discussion can be found at https://github.com/pypa/pip/issues/12330
Collecting SQLAlchemy
Using cached SQLAlchemy-2.0.28-py3-none-any.whl.metadata (9.6 kB)
Requirement already satisfied: typing-extensions>=4.6.0 in c:\progra~1\grampsaio64-5.2.0\lib\library.zip (from SQLAlchemy) (4.9.0)
Collecting greenlet!=0.4.17 (from SQLAlchemy)
Using cached greenlet-3.0.3.tar.gz (182 kB)
Installing build dependencies ... error
error: subprocess-exited-with-error
× pip subprocess to install build dependencies did not run successfully.
│ exit code: 1
╰─> [2 lines of output]
ERROR: unknown command "C:\Program Files\GrampsAIO64-5.2.0\lib\pip\__pip-runner__.py"
[end of output]
note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error
× pip subprocess to install build dependencies did not run successfully.
│ exit code: 1
╰─> See above for output.
note: This error originates from a subprocess, and is likely not a problem with pip.
Thanks. Amended the issue report on GitHub with that info.
Do you think the PIP installation failure is related to the AIO customization?
This addon is not ready for use yet 
But for some reason the unit test I added only works if I comment out the SQLAlchemy requirement (and install it manually). Any idea why & what I could add to make it pass with the comment removed?
By the way - where is the JSON file for the addon manager documented? I was not even aware of it.
Documentation for the registration files is generated from the PluginData class. It can be found here, but I see that this hasn’t been updated for v5.2 yet.
The json and the txt file before it aren’t documented. They are basically just list representations of the plugin data with abbreviated attribute names.
1 Like
I don’t think that SQLAlchemy is a pure python module. This probably means that we can’t automatically install it.
I’ll probably write a non-SQLAlchemy version for core Gramps to avoid the extra dependency. Moving the undo table into the backend was on my list of jobs for implementing multi-user access.
What do you mean by pure Python module? Yes it has some Cython extensions but it is shipped as wheel and can be pip-installed, which is what Gramps does, isn’t it? The PostgreSQL extension installs the psycopg2 module without issues, and that is not pure Python either.
I’ll probably write a non-SQLAlchemy version for core Gramps to avoid the extra dependency. Moving the undo table into the backend was on my list of jobs for implementing multi-user access.
I find that a bit ironic given that you rejected Add SQLite undo log by DavidMStraub · Pull Request #1428 · gramps-project/gramps · GitHub which intentionally used a non-SQLAlchemy implementation. I switched to SQLAlchemy for the addon because the same code can be used for SQLite and PostgreSQL, and because Gramps now supports pip-installing dependencies, which is great.
By the way I also tried to implement your suggestion to store the undo log in the main database but that was not possible without completely rewriting the transaction logic in Gramps, because the undo log is updated while a transaction is in progress and SQLite can’t have two concurrent writes. So the undo log would have to share the same transaction. This would be a breaking change on existing database backends as well.
One other challenge I have with implementing the above PR as an addon is that using the undo log for an existing datbase requires manually changing the database.txt, and a different addon is needed for different database backends (SQLite, PostgreSQL, SharedPostgreSQL) even though the undo log code is the same in all cases.
One possibility I considered is making a small pip-installable library that is shared by addons, and can be directly shared by Gramps Web API as well, so Web API can directly use the history database for any db backend (by simply overriding _create_undo_manager on database load) without the need to install an addon and change the database backend, but also without duplicating code and deviating too far from desktop Gramps (by providing the same code as plugins usable with desktop).
Just some unordered thoughts I hadn’t shared yet if @emyoulation hadn’t discovered the new repo 
1 Like
My main concern with your PR was that it continued to store the undo log on the client-side. This would make our goal of a multi-user Gramps desktop difficult. We have had so many requests for this functionality that it seems a pity not to include it in our design.
I haven’t looked at the database code closely for some years now, but my initial thought would be that updating the undo log in the same transaction as the data change would be beneficial. I can’t believe that this isn’t possible.
Another thing to consider is our support of NoSQL backends. In fact the very design of Gramps is non-relational. Interest in graph databases and hybrid models is growing.
The idea behind allowing _create_undo_manager was to allow you or anyone else to develop new functionality without the constraints of our core development cycle. I don’t want to see a proliferation a database addons though. Ideally we should have one backend per database type, rather than separate multi-user and single-user versions for example.
Perhaps overriding _create_undo_manager doesn’t give us the flexibility we require. This can always be changed.
Yes, I understand the concerns.
For a shared database, I would suggest using the SharedPostgreSQL addon. In that case the goal of my addon would be to store the history tables in the same database as the Gramps database, and I also think in that case the transaction overlap is not an issue (but I haven’t tested it yet).
For SQLite it would be possible to rewrite Gramps such that the same transaction is used for both, but that would be a much larger change than _create_undo_manager if I’m not mistaken.
For direct use in Gramps Web API, _create_undo_manager will work perfectly fine, so I am now leaning towards not continuing with a separate addon for the moment but instead moving the SQL undo manager directly into Web API to enable a history endpoint and eventually an undo/revert history section in Gramps Web.
Hopefully we can allow multi-user access to any backend. The reason for the single-user restriction was to improve the performance of the BSDDB database.
If the _create_undo_manager approach doesn’t work then I am open to consider alternatives. It may also be time to have another look at storing objects as JSON rather than pickled blobs.
I’ll release v5.2.1 over the weekend. After that, I’ll have time to start thinking about v5.3.
2 Likes
By the way, one of the lessons I learned with Web API on PostgreSQL is that some operations in Gramps become extremely slow when network lag (even a few milliseconds) is added to a SQL request, because there is often the pattern
for handle in db.iter_person_handles():
person = db.get_person_from_handle(handle)
# other stuff
rather than
for person in db.iter_people():
# other stuff
or analogously for other object types. With SQLite on a local hard drive, this does not make much difference, but it does for any type of network.
I would be willing to contribute to removing such performance bottlenecks.
3 Likes
In older code you will also see:
for handle in db.get_person_handles():
person = db.get_person_from_handle(handle)
These methods allow the results to be sorted and the objects can be modified.
The iterators are a newer feature in the API. They are faster, but the results are returned in an undefined order and the iterated objects must not be modified.
We didn’t update older code when the new iterators were introduced. I’m sure there is some code that would benefit from the updates.
1 Like