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

Emit events for data changes #252

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

simongdavies
Copy link
Member

closes #180

This change enables bindle server to emit events describing changes that have occurred to the data it manages, the feature is enabled by running the server with the flag --events. Enabling the feature causes bindle to serialize details of changes in JSON format to a file.

Copy link
Contributor

@thomastaylor312 thomastaylor312 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very helpful and extensible. Just one blocking concern on the use of async-std

@@ -75,6 +75,7 @@ openid = { version = "0.9.3", optional = true }
bcrypt = "0.10.1"
chrono = { version = "0.4.19", features = ["serde"], optional = true }
either = "1.6.1"
async-std = "1.9.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is going to require features = ["tokio1"] so that we aren't running another runtime alongside Tokio

@@ -214,6 +214,29 @@ This file can be moved from system to system, just like OpenPGP or SSH key sets.
- To create a signing key for a client, use `bindle create-key`
- By default, if Bindle does not find an existing keyring, it creates one of these when it first starts.

## Generating Change Events

Starting bindle with the `--events` flag will cause it to emit an record of the event to a file named `bindle-event.log` in the bindle data directory.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to have a note about using some sort of tool like logrotate (or we should provide one) to avoid getting an infinitely growing file


// This enum exists to enable the type of event sync to be dynamically selected at runtime depending on arguments passed
// to the program.
// It seems like the normal approach of using trait objects does not work as the trait is not object safe as it has a generic type parameter.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, you are correct here. It is the one glaring weakness of the Trait system. Generally we have not been exposing these publicly and have been putting them in the binary (as anyone can technically write something to implement the trait. Not going to block on this as we are still pre-1.0 and can break this later if needed

@@ -0,0 +1,112 @@
use async_std::fs::OpenOptions;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, now that I am looking at this again, I think all of these options are available in tokio (including the prelude below)

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

Successfully merging this pull request may close these issues.

Add Change notification to bindle server
2 participants