Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a device setting to configure what to sync #680

Closed
EvanHahn opened this issue May 27, 2024 · 4 comments · Fixed by #932
Closed

Add a device setting to configure what to sync #680

EvanHahn opened this issue May 27, 2024 · 4 comments · Fixed by #932

Comments

@EvanHahn
Copy link
Contributor

EvanHahn commented May 27, 2024

We need to persist a users's choice about what they want to sync (current options are "previews only" and "everything"). This is a device-based setting, which might be scoped to project (e.g. support different settings for each project). It shouldn't be written to the project settings of the database, which syncs with other devices.

I think this makes sense to implement on the frontend, and add an API method to change this setting.

Choices to make:

  1. Default sync config for a new project instance
    a. All new project instances start with "previews only" until configured otherwise.
    b. All new project instances start with "everything" until configured otherwise.
    c. We add a new option to api.getProject for configuring what to sync
    d. Combination of above (option + default)
  2. Method name & signature
    a. project.configureSync('previews' | 'everything')
    b. project.setArchiver(true | false)
    c. project.setBlobDownloadFilter({ variants: Array<'preview' | 'thumbnail' | 'original'> })
    d. Other ideas welcome!
@EvanHahn EvanHahn self-assigned this Aug 26, 2024
@EvanHahn EvanHahn removed their assignment Oct 10, 2024
@gmaclennan gmaclennan changed the title Add a project setting for making a device an archive device (syncing everything) Add a project setting to configure what to sync Oct 16, 2024
@gmaclennan gmaclennan changed the title Add a project setting to configure what to sync Add a device setting to configure what to sync Oct 16, 2024
@gmaclennan gmaclennan self-assigned this Oct 16, 2024
@achou11
Copy link
Member

achou11 commented Oct 17, 2024

It shouldn't be written to the project settings of the database, which syncs with other devices.

this part makes sense to me, but I don't agree with this at the moment:

I think this makes sense to implement on the frontend

It's not immediately clear why this is necessary. My initial thought was that this should be stored this in the backend via a table in the "client" database that isn't the projectSettings table (since data in that one gets synced). Think it would result in fewer implementation issues, as well as avoid UX edge cases around relying on client-side storage.


  1. Default sync config for a new project instance

for this, i would lean towards projects being configured to sync everything by default, and exposing an option to determine the sync setting upon creation.

The usage I'm imagining when creating the project looks something like this:

const manager = new MapeoManager({ ... })

// Case 1: Create a project with the default sync configuration, which is to sync everything:
manager.createProject({ ... })

// Case 2: Create a project that explicitly sets the sync setting (option name tbd)
manager.createProject({ ..., syncConfiguration: ... })

As for updating a project's sync setting, I'm wondering if the method needs to live in the manager instance instead of the project instance. Depending on how storage of this setting works (e.g. in the client database in core), it might be easier to expose this as a manager method that accepts the project id and sync setting. For example, these are the two approaches that are coming to mind:

// Approach 1: Expose a manager instance method (name and interface tbd)
manager.updateProjectConfiguration(projectId, { sync: ... })

// Approach 2: Expose a project instance method (name and interface tbd)
project.setSyncConfiguration(...)

I have a slight preference for the manager approach since this is a device-specific setting to begin with, which is usually more of a concern handled by the manager.

  1. Method name & signature

I guess my thoughts on this depend on where the method lives.

  • If it's in the manager instance, I can imagine it being similarly named to the existing methods that deal with projects e.g. createProject(), getProject(), addProject(). Maybe something like updateProject() would suffice?

  • If it's in the project instance, the ideal from a API-design perspective would be something that's exposed in the SyncApi so we could do something like project.$sync.updateConfiguration(...). Not sure if that's actually feasible, since this setting may affect other things like the BlobApi and thus may need to live as a direct method on the project instance instead. If that's the case, then something like the proposed project.configureSync() could work fine! Or maybe even something more generic like project.configure({ sync: ... })


EDIT: Also wondering if it would be helpful to name non-synced project-related settings using the "configuration" nomenclature. I find using the "settings" wording confusing because that's something that gets stored and sync across projects, whereas the feature in question is not that, but still a "setting" of some kind. I updated the naming in the example to use the "configuration" wording, to see how it feels :)

EDIT 2: I realize that I'm using the "manager" naming instead of "api" because I'm thinking about the backend implementation, which gets reflected to the client side. Just a matter of naming in this case, so replace any reference of manager with api in my examples if you're thinking about the app code I guess 😄

@EvanHahn
Copy link
Contributor Author

Default sync setting? I agree with Andrew: default to syncing everything. Make it easy to change at any time.

What is the method attached to? project.$sync feels like the most obvious spot for an external API; you're dealing with project-level sync settings, after all. I'd also be fine with any of the above options: on MapeoProject or MapeoManager/api.

What is the method signature?

I think you want to be able to specify simple filters like "download everything" or "download nothing", but also more complex queries like "only download photos" or "only download thumbnails".

This is my dream API, which I think (1) best expresses developer intent, and (2) offers the most flexibility:

// Download everything
myProject.$sync.setBlobDownloadFilters(() => true)

// Download nothing
myProject.$sync.setBlobDownloadFilters(() => false)

// Only download audio
myProject.$sync.setBlobDownloadFilters((blob) => blob.type === "audio")

// Download audio, photo previews, and photo thumbnails
myProject.$sync.setBlobDownloadFilters((blob) =>
  blob.type === "audio" ||
  (blob.type === "photo" &&
    (blob.variant === "preview" || blob.variant === "thumbnail"))
)

However, I don't know if this dream is actually possible. Is something like this fast enough? Do we need to persist filters? Does this break for some other reason?

@achou11
Copy link
Member

achou11 commented Oct 17, 2024

I'm realizing that we'll need to expose a method in the project instance one way or another, so we can dismiss the fake distinction I created in terms of where the update method lives 😅

@gmaclennan
Copy link
Member

gmaclennan commented Oct 21, 2024

Thanks all. For terminology, I suggest calling this "blob download filters", or "blob filters" for short.

Persistence: backend or frontend?
We do need to persist blob filters, for when the app is closed down and then re-opened. This state needs to be synced between the front-end and back-end, no matter where we persist this state.

If we persist on the front-end, then we need a method setBlobDownloadFilters(), and possibly a new getProject() option to remove the need to getProject then call setBlobDownloadFilters().

If we persist on the back-end, then we need two methods setBlobDownloadFilters() and getBlobDownloadFilters(). The latter will be needed when displaying the settings screen, in order to display the current filter.

I think persisting in the backend will reduce complexity / logic needed in the front-end, so I tentatively propose we persist in the back-end

Persistence: storage location
We currently have two sqlite tables which are not views on hypercore data: projectKeysTable and localDeviceInfo. projectKeysTable has a projectInfo column, which currently stores the fallback project name (project name normally comes from the synced project settings table, but if this has not yet synced, then projectKeysTable.projectInfo.name is used). We could either add another property to projectInfo or we could add an additional column.

localDeviceInfo currently has a deviceId and deviceInfo column and is designed for storing a single row. I don't see an obvious way of adding more persisted data into there.

We could create a new sqlite table for storing this information.

Alternatively we could start persisting "app config" data separately, using something like conf.

Options:

  1. Add to projectKeys table as additional info on projectInfo column.
  2. Add to projectKeys table as additional column
  3. Create new sqlite table
  4. Separate persistence mechanism

For anything that we use, we should consider handling migrations (e.g. changing of config structure). Given all of this we might want to consider using the front-end for persistence which maybe already handles migrations of config structure?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants