Skip to content

Support replication between databases #151

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

Merged
merged 19 commits into from
Nov 23, 2021

Conversation

dhirensham
Copy link
Contributor

I've had need to enable and disable replication between couch databases at runtime, so I forked the library and added it in. Seems like it's something that could be useful to others as well.
Use as follows:

var from = await client.GetOrCreateDatabaseAsync<T>>("from");
var to = await client.GetOrCreateDatabaseAsync<T>("to");
if (!await client.ReplicateAsync(new CouchReplication(from, to, true)))
{
  logger.LogError("Unable to configure replication from {from} to {to}", from, to);
}

and

var from = await client.GetOrCreateDatabaseAsync<T>>("from");
var to = await client.GetOrCreateDatabaseAsync<T>("to");
if (!await client.RemoveReplicationAsync(new CouchReplication(from, to, true)))
{
  logger.LogError("Unable to cancel replication from {from} to {to}", from, to);
}

@matteobortolazzo
Copy link
Owner

Hey, thanks for the PR! I think it's useful as well.

I added a couple of comments.

Also can you target the dev brach and update the README please? :)

@dhirensham
Copy link
Contributor Author

Hey, thanks for the PR! I think it's useful as well.

I added a couple of comments.

Also can you target the dev brach and update the README please? :)

I can't seem to change the target branch, if the option is available for you, could you please change it to dev for me?

@matteobortolazzo matteobortolazzo changed the base branch from master to dev November 12, 2021 20:40
@matteobortolazzo
Copy link
Owner

Target branch updated

@dhirensham
Copy link
Contributor Author

README updated with sample usage

@matteobortolazzo
Copy link
Owner

Nice!

A note, I am checking the documentation , and seems like there are more properties to have 100% API coverage, and for example, the selector can be replaced with a function since it's the same used for queries and other stuff.

Also since target and source are mandatory I would move them directly to the method with an optional ReplicationOptions object for all the other stuff.

This is because I want to keep a consistent API with the rest of the problem.

Let me know if you want to give it a try.
Otherwise I can merge and I will revise later when I have time because I merge into master

@dhirensham
Copy link
Contributor Author

I'll move the source and target out of the ReplicationOptions object and look at the other replication properties - I only implemented the ones that I needed, so I'll look into adding in support for the other features as well - I'd like to be able to test them first so might want to hold off on those until I'm able to do that. But I'd rather you only merge once the API's been made more consistent - should be able to update the PR tomorrow with that change

@dhirensham
Copy link
Contributor Author

@matteobortolazzo I've made the changes to keep the API more consistent with the existing calls, so happy for this to get merged if you are. I'll look at the remaining properties as soon as I have a chance and send through a separate PR for that.

@matteobortolazzo
Copy link
Owner

Sorry the late answer, I'll merge it :) thanks a lot!!!

@matteobortolazzo matteobortolazzo merged commit aa1a9ea into matteobortolazzo:dev Nov 23, 2021
@matteobortolazzo
Copy link
Owner

@dhirensham One thing, please add tests in the next PR :)

@matteobortolazzo
Copy link
Owner

And I think CouchReplication? replication = null, bool persistent = true should be IMO CouchReplicationOptions? options = null with persistent = true in the options. Do you agree?

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.

3 participants