You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by robertkowalski <gi...@git.apache.org> on 2014/09/01 19:12:54 UTC
[GitHub] couchdb-fauxton pull request: Replication fixes
GitHub user robertkowalski opened a pull request:
https://github.com/apache/couchdb-fauxton/pull/52
Replication fixes
A sliceoff from https://github.com/apache/couchdb-fauxton/pull/50
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/robertkowalski/couchdb-fauxton COUCHDB-2244--replication
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-fauxton/pull/52.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #52
----
commit c8fa7ddb7f8e4adfe269b727f7a4db489d50a69c
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2014-08-31T20:58:39Z
Convert tabs to spaces, remove trailing whitespaces
commit a32335473aea6b84a930385c9fee9c4dd493c8ce
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2014-08-31T22:12:32Z
Enable access on replication for non-admins
COUCHDB-2244
commit 5ea741531c0d58a28442abcad68d9cf8542a2515
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2014-08-31T23:36:00Z
Allow replications to existing databases
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Replication fixes
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/52#discussion_r16983406
--- Diff: app/addons/replication/views.js ---
@@ -200,14 +205,23 @@ function(app, FauxtonAPI, Components, replication) {
View.ReplicationList = FauxtonAPI.View.extend({
tagName: "ul",
initialize: function(){
+ if (!this.isAdmin) {
--- End diff --
I don't like having all these `isAdmin` checks. I think what would be better is to create a non admin `ReplicationList` view that is an extension of `ReplicationList` just with all those methods returning nothing. Then the view that adds the `ReplicationList` view can decide which view to add.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Replication fixes
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/52#issuecomment-54377109
I think this is much better now, can you take a second look?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Replication fixes
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/52#discussion_r16983224
--- Diff: app/addons/replication/route.js ---
@@ -32,19 +31,25 @@ function(app, FauxtonAPI, Replication, Views) {
{"name": "Replicate changes from: ", "link": "replication"}
],
defaultView: function(dbname){
- this.databases = new Replication.DBList({});
- this.tasks = new Replication.Tasks({id: "ReplicationTasks"});
+ var isAdmin = FauxtonAPI.session.isUserAdmin();
+
+ this.databases = new Replication.DBList({});
+ if (isAdmin) {
+ this.tasks = new Replication.Tasks({id: "ReplicationTasks"});
+ }
+
this.replication = new Replication.Replicate({});
- this.setView("#dashboard-content", new Views.ReplicationForm({
+ this.setView("#dashboard-content", new Views.ReplicationForm({
selectedDB: dbname ||"",
- collection: this.databases,
- status: this.tasks
- }));
+ collection: this.databases,
+ status: this.tasks,
--- End diff --
What happens if we pass in an `undefined` value here? If the user isn't an admin this.tasks will be undefined, will the `ReplicationForm` still work?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Replication fixes
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/52#discussion_r17231150
--- Diff: app/addons/replication/route.js ---
@@ -32,19 +31,30 @@ function(app, FauxtonAPI, Replication, Views) {
{"name": "Replicate changes from: ", "link": "replication"}
],
defaultView: function(dbname){
- this.databases = new Replication.DBList({});
- this.tasks = new Replication.Tasks({id: "ReplicationTasks"});
+ var isAdmin = FauxtonAPI.session.isAdmin();
+
+ this.databases = new Replication.DBList({});
this.replication = new Replication.Replicate({});
- this.setView("#dashboard-content", new Views.ReplicationForm({
+
+ if (isAdmin) {
+ this.tasks = new Replication.Tasks({id: "ReplicationTasks"});
+ this.setView("#dashboard-content", new Views.ReplicationFormForAdmins({
+ selectedDB: dbname ||"",
+ collection: this.databases,
+ status: this.tasks
+ }));
+ return;
+ }
+ this.setView("#dashboard-content", new Views.ReplicationForm({
selectedDB: dbname ||"",
- collection: this.databases,
- status: this.tasks
- }));
+ collection: this.databases,
+ status: this.tasks
--- End diff --
I would prefer that this is set `this.tasks = [];` at the beginning of this function. So we at least has some sort of value for it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Replication fixes
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/52#discussion_r17078620
--- Diff: app/addons/replication/views.js ---
@@ -200,14 +205,23 @@ function(app, FauxtonAPI, Components, replication) {
View.ReplicationList = FauxtonAPI.View.extend({
tagName: "ul",
initialize: function(){
+ if (!this.isAdmin) {
--- End diff --
yes, good idea!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Replication fixes
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/52#issuecomment-54143493
@robertkowalski this is great work. Just a few small code suggestions.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Replication fixes
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/52#issuecomment-54801698
Just a small suggestion. Otherwise +1 this is good.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Replication fixes
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/52#discussion_r16983521
--- Diff: app/core/couchdbSession.js ---
@@ -27,6 +27,11 @@ function (FauxtonAPI) {
};
},
+ isUserAdmin: function () {
--- End diff --
This is great. Since everywhere you use `isAdmin`, why not just call this `isAdmin`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Replication fixes
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski closed the pull request at:
https://github.com/apache/couchdb-fauxton/pull/52
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Replication fixes
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/52#discussion_r16983755
--- Diff: app/addons/replication/route.js ---
@@ -32,19 +31,25 @@ function(app, FauxtonAPI, Replication, Views) {
{"name": "Replicate changes from: ", "link": "replication"}
],
defaultView: function(dbname){
- this.databases = new Replication.DBList({});
- this.tasks = new Replication.Tasks({id: "ReplicationTasks"});
+ var isAdmin = FauxtonAPI.session.isUserAdmin();
+
+ this.databases = new Replication.DBList({});
+ if (isAdmin) {
+ this.tasks = new Replication.Tasks({id: "ReplicationTasks"});
+ }
+
this.replication = new Replication.Replicate({});
- this.setView("#dashboard-content", new Views.ReplicationForm({
+ this.setView("#dashboard-content", new Views.ReplicationForm({
selectedDB: dbname ||"",
- collection: this.databases,
- status: this.tasks
- }));
+ collection: this.databases,
+ status: this.tasks,
--- End diff --
Suddenly, it works. Don't ask me how, but replication starts and running. You just don't see the active tasks and any progress by oblivious reasons.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: Replication fixes
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/52#issuecomment-54816945
Merged! Thanks for the feedback!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---