FamilySearch integration: architectural & govenernce discussion

[DavidMStraub]

Hi all,

I’d like to discuss some questions about the upcoming FamilySearch integration in Gramps.

I reviewed the code in the master branch. The implementation is certainly well done technically! What surprised me though was that it’s not an addon – not even a core addon – but it’s deeply integrated into the core library. There is even a change to the database schema to accommodate a new field. I have not found any way to disable the integration, hiding the new fields for users that don’t want to use the service.

Here is what I would suggest.

Allow users to disable the integration. I know it’s not sending any data when users are not logged in, but I think users should have the possibility to completely disable it, which means not showing a logo, button, or “FamilySearch” string anywhere in the UI except the preferences dialog. And this should be the default. I believe this part is crucial to keep our independence as a free and open source project.

Reconsider the database schema change. Gramps already has fields in its data model, e.g. attributes, that can be used to track persistent identifiers. Adding an additional column to the person table just to track the unique ID in the system of a specific provider is something I find highly problematic.

Allow Gramps Web specific code in core. Not that this has ever been requested (let alone rejected), but I believe that if we allow a web service that is not part of our open source project to become integrated into Gramps core, we necessarily should add Gramps Web code (such as the sync addon) into core as well. Otherwise we would promote an external project to be more visible and better integrated than our own community project.

To be clear, I’m not opposed to the FS integration and my actual goal of this investigation was to start investigating how it can be leveraged in Gramps Web, but I think the points above need discussion.

Thanks!

[Nick-Hall]

@DavidMStraub

Allow users to disable the integration.

I believe that this was implemented in the latest update today.

@DavidMStraub

Reconsider the database schema change.

This was requested by Doug and I agreed with the decision.

@DavidMStraub

Allow Gramps Web specific code in core.

I am certainly open to including Gramps Web code in core. In the case of addons, we are tending to move them out of core.

[dsblank]

@Nick-Hall

This was requested by Doug and I agreed with the decision.

I did what?! I think an attribute would be preferable. If I said something that was interpreted to be changing the schema, I think I must have misinterpreted. What I did say initially:

Architecture Concerns

4. Integration belongs in a plugin, not gramps/gui/ core
All of the FamilySearch code is in gramps/gui/fs/, making it a permanent part of the core application for all users, including those who don’t use or want FamilySearch. The existing pattern for this kind of integration is a Gramps plugin/addon (see gramps/plugins/). This keeps optional functionality optional and doesn’t impose new dependencies on all users.

5. Core file modifications for an optional feature
grampsgui.py, viewmanager.py, displaystate.py, editperson.py, configure.py, and statusbar.py are all modified to wire in FamilySearch. If FamilySearch were a plugin, none of these files would need touching.

FamilySearch Integration (#2176)

[dsblank]

@Nick-Hall

I am certainly open to including Gramps Web code in core. In the case of addons, we are tending to move them out of core.

It would be nice to have a new plugin type (like “integration”) that would work with FS, or gramps web. (In fact I have started working on an alternative to gramps web).

[DavidMStraub]

Thanks!

Concerning Gramps Web, I’ll review what can be done and open a GEP before writing any code.

[comeng]

Allow users to disable the integration. I know it’s not sending
any data when users are not logged in, but I think users should have the
possibility to completely disable it,

In my view the default should be disabled and the users have to go
hunting for it if they want it.
Do not give me and many others something else we have to remember to
turn off.

phil

[Nick-Hall]

@dsblank

I did what?! I think an attribute would be preferable.

Probably a misinterpretation on my part. I was remembering this review comment.

10. Strings-as-status storage in datab_familysearch.py
    Sync metadata (FSFTID, sync timestamps, etc.) is stored as a JSON blob in a Person attribute (_GRAMPSFS_SYNC_STATUS). The code itself notes this is a workaround. This pollutes the genealogical data model and makes it hard to query. This needs a proper design decision.

The data structure added contains more than an ID. We decided that rather store a blob in an attribute, it would be better to create a new object.

@dsblank

4. Integration belongs in a plugin, not gramps/gui/ core

The original code written by @SourceAirbender started life as an addon. The decision to move the code to core was made following one of our early project meetings. In addition to the technical aspects there were also certification issues to consider.

[Nick-Hall]

@comeng

In my view the default should be disabled and the users have to go
hunting for it if they want it.

Yes. The integration is disabled by default.

[emyoulation]

@DavidMStraub

Allow users to disable the integration. I know it’s not sending any data when users are not logged in, but I think users should have the possibility to completely disable it, which means not showing a logo, button, or “FamilySearch” string anywhere in the UI except the preferences dialog. And this should be the default. I believe this part is crucial to keep our independence as a free and open source project.

I agree. As suggested before.

Since the integration creates a lot of Input Options, that subsection of the Edit → Preferences → Data tab is where the switch should live.

And, like it or not, FamilySearch and the LDS tab data are part of the same data ecology. (User accounts on FamilySearch can sign in with their Church Membership. Which open access to additional Ordinance related options. If we are not looking at the ways the LDS tabs can leverage the new FamilySearch access, then we are shooting ourselves in the foot. The path to improving the FamilySearch integration is the adoption of Gramps as a tool that is useful to the LDS mission. We can get more engagement from their organization if ordinances scraping and posting is improved too.)

So the feature enable/disable FamilySearch and LDS tabs should be adjacent in the interface.

[Nick-Hall]

@emyoulation

If we are not looking at the ways the LDS tabs can leverage the new FamilySearch access, then we are shooting ourselves in the foot.

We are not allowed access to the LDS ordinance part the the FamilySearch API at this time. This may be possible in the future, but we need to take one step at a time.

[Nick-Hall]

@DavidMStraub

Gramps already has fields in its data model, e.g. attributes, that can be used to track persistent identifiers.

Attributes were always intended to store genealogical data. Using them to store identifiers has always been regarded as a workaround.

The new object is a good idea and I can see it being used by other integrations in the future.

[DavidMStraub]

The new object is a good idea and I can see it being used by other integrations in the future.

Which new object?

[emyoulation]

@DavidMStraub

Which new object?

Perhaps Nick was referring to this earlier posting? :

@Nick-Hall

We decided that rather store a blob in an attribute, it would be better to create a new object.

[aviansid]

The new secondary object is in gen/lib/fs/familysearchsync.py:

            "properties": {
                "_class": {"enum": [cls.__name__]},
                "fsid": {"type": ["string", "null"], "title": _("FamilySearch ID")},
                "is_root": {"type": "boolean", "title": _("Is root")},
                "status_ts": {
                    "type": ["integer", "null"],
                    "title": _("Status timestamp"),
                },
                "confirmed_ts": {
                    "type": ["integer", "null"],
                    "title": _("Confirmed timestamp"),
                },
                "gramps_modified_ts": {
                    "type": ["integer", "null"],
                    "title": _("Gramps modified timestamp"),
                },
                "fs_modified_ts": {
                    "type": ["integer", "null"],
                    "title": _("FamilySearch modified timestamp"),
                },
                "essential_conflict": {
                    "type": "boolean",
                    "title": _("Essential conflict"),
                },
                "conflict": {"type": "boolean", "title": _("Conflict")},
            },

[Nick-Hall]

Yes. I can see this being extended in the future to become a general Sync object.

[DavidMStraub]

Future meaning after 6.1? What is the roadmap for 6.2 then?

[emyoulation]

@Nick-Hall

Yes. I can see this being extended in the future to become a general Sync object.

So a Delta or Diff object system would exist? Something we could use to build a worklist? (Like extending the Import Merge addon tool by @prculley. So that we could save a worklist and manage the workload instead of being crushed by it, having to resolve everything NOW in a semi-modal dialog?)

[dsblank]

@emyoulation

So a Delta or Diff object system would exist? Something we could use to build a worklist? (Like extending the Import Merge addon tool by @prculley. So that we could save a worklist and manage the workload instead of being crushed by it, having to resolve everything NOW in a semi-modal dialog?)

Very good idea of having a queue of merge conflicts to work through. But you don’t have to extend prculley’s addon as it was built on my diff infrastructure. We just need to be able to load and save the queue.

[1] - https://www.gramps-project.org/wiki/index.php/Addon:Database_Differences_Report
[2] - gramps/gramps/gen/merge/diff.py at master · gramps-project/gramps · GitHub
[3] - gramps/gramps/gen/merge/diff.py at master · gramps-project/gramps · GitHub

[DavidMStraub]

@Nick-Hall

Yes. I can see this being extended in the future to become a general Sync object.

This sounds good but I don’t think it will work in practice.

Different tools will have very different requirements for a sync object (this is obvious from comparing the FS feature with Gramps Web sync, for instance).

Plus, we can’t have one object if we want to allow use of multiple sync tools. And I don’t think we want to bar anyone enabling the FS integration from using Gramps Web simultaneously!

So I don’t think a single object is feasible. Instead, I’d say we have set a precedent by adding a metadata object for an external sync tool, and we can add similar ones for other tools in the future.

[Nick-Hall]

@DavidMStraub

Plus, we can’t have one object if we want to allow use of multiple sync tools.

Correct. We would have to allow many objects. One for each integration.

Perhaps all Sync objects could inherit from a common base class? I’m sure that there will be options to consider.