U1DB

Merge lp:~thisfred/u1db/remove-local-gen-before-sync into lp:u1db

  1. remove-local-gen-before-sync
  2. Merge into trunk
Proposed by Eric Casteleijn
Status: Rejected
Rejected by: Eric Casteleijn
Proposed branch: lp:~thisfred/u1db/remove-local-gen-before-sync
Merge into: lp:u1db
Diff against target: 164 lines (+18/-23)
5 files modified
include/u1db/u1db_internal.h (+1/-2)
src/u1db.c (+1/-1)
src/u1db_sync_target.c (+5/-5)
u1db/tests/c_backend_wrapper.pyx (+3/-5)
u1db/tests/test_sync.py (+8/-10)
To merge this branch: bzr merge lp:~thisfred/u1db/remove-local-gen-before-sync
Reviewer Review Type Date Requested Status
Samuele Pedroni Disapprove
dobey (community) Needs Fixing
Review via email: mp+119642@code.launchpad.net

Commit message

removed unused parameter

Description of the change

removed unused parameter

To post a comment you must log in.
Revision history for this message
Samuele Pedroni (pedronis) :
review: Approve
Revision history for this message
dobey (dobey) wrote :

01:31:04 O: [ 33%] Building C object src/CMakeFiles/u1db.dir/u1db.c.o
01:31:04 E: /mnt/tarmac/cache/u1db/trunk/src/u1db.c: In function ‘u1db_sync’:
01:31:04 E: /mnt/tarmac/cache/u1db/trunk/src/u1db.c:2092:5: error: too many arguments to function ‘u1db__sync_db_to_target’
01:31:04 E: In file included from /mnt/tarmac/cache/u1db/trunk/src/u1db.c:28:0:
01:31:04 E: /mnt/tarmac/cache/u1db/trunk/include/u1db/u1db_internal.h:544:5: note: declared here
01:31:04 E: make[3]: *** [src/CMakeFiles/u1db.dir/u1db.c.o] Error 1
01:31:04 E: make[2]: *** [src/CMakeFiles/u1db.dir/all] Error 2
01:31:04 E: make[1]: *** [CMakeFiles/check-valgrind.dir/rule] Error 2
01:31:04 E: make: *** [check-valgrind] Error 2

review: Needs Fixing
lp:~thisfred/u1db/remove-local-gen-before-sync updated
379. By Eric Casteleijn

remerged trunk:

380. By Eric Casteleijn

fixed leftover call:

Revision history for this message
Eric Casteleijn (thisfred) wrote :

fixed

Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 8/15/2012 12:57 AM, Eric Casteleijn wrote:
> Eric Casteleijn has proposed merging
> lp:~thisfred/u1db/remove-local-gen-before-sync into lp:u1db.
>
> Requested reviews: Ubuntu One hackers (ubuntuone-hackers)
>
> For more details, see:
> https://code.launchpad.net/~thisfred/u1db/remove-local-gen-before-sync/+merge/119642
>
> removed unused parameter
>

This was meant to be an OUT parameter for sync (the return value from
the Python code).

The idea is that you want to be able to get the local generation of
the database after the sync, so that you can call
'whats_changed(before_sync_gen)' without having a race condition.

It may be that we are missing tests doing that sort of thing, so the
variable never got set, because we didn't realize that we should be.

But it *was* meant to be used.

Is there some other change that invalidates this?

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlArVA4ACgkQJdeBCYSNAAO1PwCfZ5fBV5pC1d6qIpkXevcdSItV
P/MAn0A5XKtcqWN7+r5+XEgCUG6vAtGO
=0eye
-----END PGP SIGNATURE-----

Revision history for this message
Samuele Pedroni (pedronis) wrote :

so it seems it had a thought use

review: Abstain
Revision history for this message
Samuele Pedroni (pedronis) :
review: Disapprove

Unmerged revisions

380. By Eric Casteleijn

fixed leftover call:

379. By Eric Casteleijn

remerged trunk:

378. By Eric Casteleijn

removed unused parameter

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/u1db/u1db_internal.h'
2--- include/u1db/u1db_internal.h 2012-07-26 18:07:54 +0000
3+++ include/u1db/u1db_internal.h 2012-08-15 02:11:18 +0000
4@@ -541,7 +541,6 @@
5 /**
6 * Sync a database with a sync target.
7 */
8-int u1db__sync_db_to_target(u1database *db, u1db_sync_target *target,
9- int *local_gen_before_sync);
10+int u1db__sync_db_to_target(u1database *db, u1db_sync_target *target);
11
12 #endif // U1DB_INTERNAL_H
13
14=== modified file 'src/u1db.c'
15--- src/u1db.c 2012-08-14 23:05:22 +0000
16+++ src/u1db.c 2012-08-15 02:11:18 +0000
17@@ -2089,7 +2089,7 @@
18 status = u1db__get_generation(db, &local_gen);
19 if (status != U1DB_OK)
20 return status;
21- status = u1db__sync_db_to_target(db, target, &local_gen);
22+ status = u1db__sync_db_to_target(db, target);
23 finish:
24 if (target != NULL) {
25 u1db__free_sync_target(&target);
26
27=== modified file 'src/u1db_sync_target.c'
28--- src/u1db_sync_target.c 2012-07-06 20:48:40 +0000
29+++ src/u1db_sync_target.c 2012-08-15 02:11:18 +0000
30@@ -584,8 +584,7 @@
31
32
33 int
34-u1db__sync_db_to_target(u1database *db, u1db_sync_target *target,
35- int *local_gen_before_sync)
36+u1db__sync_db_to_target(u1database *db, u1db_sync_target *target)
37 {
38 int status;
39 struct _whats_changed_doc_ids_state to_send_state = {0};
40@@ -598,9 +597,10 @@
41 char *target_trans_id = NULL;
42 int target_gen, local_gen;
43 int local_gen_known_by_target, target_gen_known_by_local;
44+ int local_gen_before_sync;
45
46 // fprintf(stderr, "Starting\n");
47- if (db == NULL || target == NULL || local_gen_before_sync == NULL) {
48+ if (db == NULL || target == NULL) {
49 // fprintf(stderr, "DB, target, or local are NULL\n");
50 status = U1DB_INVALID_PARAMETER;
51 goto finish;
52@@ -640,7 +640,7 @@
53 // logic, no need to look for more information.
54 goto finish;
55 }
56- *local_gen_before_sync = local_gen;
57+ local_gen_before_sync = local_gen;
58 return_doc_state.db = db;
59 return_doc_state.target_uid = target_uid;
60 return_doc_state.num_inserted = 0;
61@@ -663,7 +663,7 @@
62 target_trans_id_known_by_local);
63 if (status != U1DB_OK) { goto finish; }
64 if (return_doc_state.num_inserted > 0 &&
65- ((*local_gen_before_sync + return_doc_state.num_inserted)
66+ ((local_gen_before_sync + return_doc_state.num_inserted)
67 == local_gen))
68 {
69 status = target->record_sync_info(
70
71=== modified file 'u1db/tests/c_backend_wrapper.pyx'
72--- u1db/tests/c_backend_wrapper.pyx 2012-08-14 23:05:22 +0000
73+++ u1db/tests/c_backend_wrapper.pyx 2012-08-15 02:11:18 +0000
74@@ -247,8 +247,7 @@
75 int *wildcard)
76 int u1db__get_sync_target(u1database *db, u1db_sync_target **sync_target)
77 int u1db__free_sync_target(u1db_sync_target **sync_target)
78- int u1db__sync_db_to_target(u1database *db, u1db_sync_target *target,
79- int *local_gen_before_sync) nogil
80+ int u1db__sync_db_to_target(u1database *db, u1db_sync_target *target) nogil
81
82 int u1db__sync_exchange_insert_doc_from_source(u1db_sync_exchange *se,
83 u1db_document *doc, int source_gen, const_char_ptr trans_id)
84@@ -1454,14 +1453,13 @@
85 """Sync the data between a CDatabase and a CSyncTarget"""
86 cdef CDatabase cdb
87 cdef CSyncTarget ctarget
88- cdef int local_gen = 0, status
89+ cdef int status
90
91 cdb = db
92 ctarget = target
93 with nogil:
94- status = u1db__sync_db_to_target(cdb._db, ctarget._st, &local_gen)
95+ status = u1db__sync_db_to_target(cdb._db, ctarget._st)
96 handle_status("sync_db_to_target", status)
97- return local_gen
98
99
100 def create_http_sync_target(url):
101
102=== modified file 'u1db/tests/test_sync.py'
103--- u1db/tests/test_sync.py 2012-08-09 20:46:13 +0000
104+++ u1db/tests/test_sync.py 2012-08-15 02:11:18 +0000
105@@ -503,7 +503,7 @@
106 self.assertEqual(expected, log)
107
108 def test_sync_tracks_db_generation_of_other(self):
109- self.assertEqual(0, self.sync(self.db1, self.db2))
110+ self.sync(self.db1, self.db2)
111 self.assertEqual(
112 (0, ''), self.db1._get_replica_gen_and_trans_id('test2'))
113 self.assertEqual(
114@@ -704,7 +704,7 @@
115
116 def test_sync_puts_changes(self):
117 doc = self.db1.create_doc_from_json(simple_doc)
118- self.assertEqual(1, self.sync(self.db1, self.db2))
119+ self.sync(self.db1, self.db2)
120 self.assertGetDoc(self.db2, doc.doc_id, doc.rev, simple_doc, False)
121 self.assertEqual(1, self.db1._get_replica_gen_and_trans_id('test2')[0])
122 self.assertEqual(1, self.db2._get_replica_gen_and_trans_id('test1')[0])
123@@ -717,7 +717,7 @@
124 def test_sync_pulls_changes(self):
125 doc = self.db2.create_doc_from_json(simple_doc)
126 self.db1.create_index('test-idx', 'key')
127- self.assertEqual(0, self.sync(self.db1, self.db2))
128+ self.sync(self.db1, self.db2)
129 self.assertGetDoc(self.db1, doc.doc_id, doc.rev, simple_doc, False)
130 self.assertEqual(1, self.db1._get_replica_gen_and_trans_id('test2')[0])
131 self.assertEqual(1, self.db2._get_replica_gen_and_trans_id('test1')[0])
132@@ -740,8 +740,7 @@
133 return
134 self.db1.create_doc_from_json(simple_doc)
135
136- self.assertEqual(0, self.sync(self.db1, self.db2,
137- trace_hook=before_get_docs))
138+ self.sync(self.db1, self.db2, trace_hook=before_get_docs)
139 self.assertLastExchangeLog(self.db2,
140 {'receive': {'docs': [], 'last_known_gen': 0},
141 'return': {'docs': [(doc.doc_id, doc.rev)],
142@@ -760,8 +759,7 @@
143 if state != 'record_sync_info':
144 return
145 self.fail('SyncTarget.record_sync_info was called')
146- self.assertEqual(1, self.sync(self.db1, self.db2,
147- trace_hook=no_record_sync_info))
148+ self.sync(self.db1, self.db2, trace_hook=no_record_sync_info)
149 self.assertEqual(
150 1,
151 self.db2._get_replica_gen_and_trans_id(self.db1._replica_uid)[0])
152@@ -769,9 +767,9 @@
153 def test_sync_ignores_convergence(self):
154 doc = self.db1.create_doc_from_json(simple_doc)
155 self.db3 = self.create_database('test3')
156- self.assertEqual(1, self.sync(self.db1, self.db3))
157- self.assertEqual(0, self.sync(self.db2, self.db3))
158- self.assertEqual(1, self.sync(self.db1, self.db2))
159+ self.sync(self.db1, self.db3)
160+ self.sync(self.db2, self.db3)
161+ self.sync(self.db1, self.db2)
162 self.assertLastExchangeLog(self.db2,
163 {'receive': {'docs': [(doc.doc_id, doc.rev)],
164 'source_uid': 'test1',

Subscribers

People subscribed via source and target branches