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

Changes to database required for syncing #61

Open
ErikBjare opened this issue Aug 28, 2019 · 7 comments
Open

Changes to database required for syncing #61

ErikBjare opened this issue Aug 28, 2019 · 7 comments

Comments

@ErikBjare
Copy link
Member

A couple of things:

  • We need to be able to create instances of the database that do not migrate/update like the local instance does (to ensure that they are readable by versions of aw-server-rust that are not up-to-date)
  • We need a way to enable read-only DB access

Thoughts @johan-bjareholt?

@johan-bjareholt
Copy link
Member

We need to be able to create instances of the database that do not migrate/update like the local instance does (to ensure that they are readable by versions of aw-server-rust that are not up-to-date)

Maybe look at the schema version and if the schema version is not the same as the current one we should make a user-visible warning that the db from another instance is older/newer?

We need a way to enable read-only DB access

Agreed!

@johan-bjareholt
Copy link
Member

johan-bjareholt commented Aug 28, 2019

Maybe also make a unique identifier for each computer?
Or should we continue using hostname and simply refuse if two machines have the same hostname?

I'd prefer using hostname because it's more human readable, but I can also see the issue with it so my stance on that question is neutral.

@johan-bjareholt
Copy link
Member

I realized that the whole mpsc thread model is a bit excessive if we are to only have read access to the database, so I rewrote the datastore code to be able to more nicely work without the worker.

#65

I would suggest to adapt the syncing code to create a read-only SQLite connection itself instead of going through the worker by only using the DatastoreInstance.

@ErikBjare
Copy link
Member Author

ErikBjare commented Sep 16, 2019

Maybe also make a unique identifier for each computer?

I think we should at some point as people have already had issues with this (https://forum.activitywatch.net/t/remove-host-computer/358). Syncthing does it by generating a GUID for every host, possibly derived from the hosts public key.

I opened an issue about it: ActivityWatch/activitywatch#302

@johan-bjareholt
Copy link
Member

We need to be able to create instances of the database that do not migrate/update like the local instance does (to ensure that they are readable by versions of aw-server-rust that are not up-to-date)

Fixed in #76

@ErikBjare
Copy link
Member Author

Fixed in #76

I appreciate it, but I kinda think that the solution you proposed was better (and is how Syncthing deals with nodes of different versions):

Maybe look at the schema version and if the schema version is not the same as the current one we should make a user-visible warning that the db from another instance is older/newer?

@johan-bjareholt
Copy link
Member

I appreciate it, but I kinda think that the solution you proposed was better (and is how Syncthing deals with nodes of different versions)

If you look at the change you can see that this does both, the added "migrate_enabled" parameter does the following:

  • If set to true, same as previous behavior
  • If set to false it does not migrate. If the version is incompatible it returns a "OldDbVersion" error so we can notify the user gracefully to update

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

No branches or pull requests

2 participants