Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(920)

Issue 311210043: <#11781292 Add support for non-incremental groups push

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 months, 4 weeks ago by imysak
Modified:
4 months, 1 week ago
Reviewers:
sfruhwald, ondrejnovak, pjo
CC:
connector-cr_google.com, sfruhwald
Visibility:
Public.

Description

<#11781292 Add support for non-incremental groups push

Patch Set 1 #

Total comments: 1

Patch Set 2 : #11781292 Add support for non-incremental groups push #

Patch Set 3 : #11781292 Add support for non-incremental groups push #

Total comments: 14

Patch Set 4 : #11781292 Add support for non-incremental groups push #

Patch Set 5 : #11781292 Add support for non-incremental groups push #

Total comments: 4

Patch Set 6 : #11781292 Add support for non-incremental groups push #

Total comments: 1

Patch Set 7 : #11781292 Add support for non-incremental groups push #

Total comments: 8
Unified diffs Side-by-side diffs Delta from patch set Stats (+204 lines, -82 lines) Patch
M src/com/google/enterprise/adaptor/DocIdSender.java View 1 2 3 7 chunks +35 lines, -11 lines 6 comments Download
M src/com/google/enterprise/adaptor/GsaFeedFileSender.java View 2 chunks +5 lines, -3 lines 1 comment Download
M src/com/google/enterprise/adaptor/Journal.java View 1 2 3 4 5 6 4 chunks +16 lines, -2 lines 0 comments Download
M test/com/google/enterprise/adaptor/DocIdSenderTest.java View 1 2 3 4 5 8 chunks +102 lines, -55 lines 1 comment Download
M test/com/google/enterprise/adaptor/GsaFeedFileSenderTest.java View 1 6 chunks +46 lines, -11 lines 0 comments Download

Messages

Total messages: 21
imysak
8 months, 4 weeks ago (2016-10-25 23:38:35 UTC) #1
JohnL
https://codereview.appspot.com/311210043/diff/1/src/com/google/enterprise/adaptor/DocIdPusher.java File src/com/google/enterprise/adaptor/DocIdPusher.java (right): https://codereview.appspot.com/311210043/diff/1/src/com/google/enterprise/adaptor/DocIdPusher.java#newcode172 src/com/google/enterprise/adaptor/DocIdPusher.java:172: boolean caseSensitive, boolean incremental) throws InterruptedException; You can't change ...
8 months, 4 weeks ago (2016-10-26 01:26:59 UTC) #2
pjo
+1 to being backwards compatible. There are dozens of V4 connectors in the world. Updating ...
8 months, 4 weeks ago (2016-10-26 04:19:01 UTC) #3
sfruhwald
+1 Also please check if each of my previous comments for this CL have been ...
8 months, 4 weeks ago (2016-10-26 04:27:22 UTC) #4
imysak
this CL doesn't change public interfaces, fix full push group deffinition, we don't need to ...
8 months, 2 weeks ago (2016-11-03 23:08:53 UTC) #5
imysak
refactoring affected tests
8 months, 2 weeks ago (2016-11-04 01:00:21 UTC) #6
sfruhwald
Thank you! https://codereview.appspot.com/311210043/diff/40001/src/com/google/enterprise/adaptor/DocIdSender.java File src/com/google/enterprise/adaptor/DocIdSender.java (right): https://codereview.appspot.com/311210043/diff/40001/src/com/google/enterprise/adaptor/DocIdSender.java#newcode229 src/com/google/enterprise/adaptor/DocIdSender.java:229: return pushGroupDefinitionsInternal(defs, caseSensitive, true, handler); Why not ...
8 months, 1 week ago (2016-11-11 01:43:29 UTC) #7
imysak
Thanks Szabi, applying changes following comments
8 months, 1 week ago (2016-11-11 18:53:31 UTC) #8
imysak
https://codereview.appspot.com/311210043/diff/40001/src/com/google/enterprise/adaptor/DocIdSender.java File src/com/google/enterprise/adaptor/DocIdSender.java (right): https://codereview.appspot.com/311210043/diff/40001/src/com/google/enterprise/adaptor/DocIdSender.java#newcode229 src/com/google/enterprise/adaptor/DocIdSender.java:229: return pushGroupDefinitionsInternal(defs, caseSensitive, true, handler); On 2016/11/11 01:43:28, sfruhwald ...
8 months, 1 week ago (2016-11-11 18:55:33 UTC) #9
sfruhwald
https://codereview.appspot.com/311210043/diff/40001/test/com/google/enterprise/adaptor/DocIdSenderTest.java File test/com/google/enterprise/adaptor/DocIdSenderTest.java (right): https://codereview.appspot.com/311210043/diff/40001/test/com/google/enterprise/adaptor/DocIdSenderTest.java#newcode53 test/com/google/enterprise/adaptor/DocIdSenderTest.java:53: private static final Map<GroupPrincipal, Collection<Principal>> SAMPLE_DATE On 2016/11/11 18:55:33, ...
8 months ago (2016-11-18 23:51:38 UTC) #10
imysak
Applying fixes
8 months ago (2016-11-19 00:07:06 UTC) #11
sfruhwald
https://codereview.appspot.com/311210043/diff/80001/src/com/google/enterprise/adaptor/Journal.java File src/com/google/enterprise/adaptor/Journal.java (right): https://codereview.appspot.com/311210043/diff/80001/src/com/google/enterprise/adaptor/Journal.java#newcode342 src/com/google/enterprise/adaptor/Journal.java:342: * @return <code>true</code> means that full push was started ...
7 months, 3 weeks ago (2016-11-29 22:19:59 UTC) #12
imysak
Applying fixes
7 months, 3 weeks ago (2016-11-30 00:45:38 UTC) #13
sfruhwald
LGTM (fix that small typo pls) https://codereview.appspot.com/311210043/diff/100001/src/com/google/enterprise/adaptor/Journal.java File src/com/google/enterprise/adaptor/Journal.java (right): https://codereview.appspot.com/311210043/diff/100001/src/com/google/enterprise/adaptor/Journal.java#newcode399 src/com/google/enterprise/adaptor/Journal.java:399: * @return <code>true</code> ...
7 months, 3 weeks ago (2016-11-30 01:01:20 UTC) #14
imysak
Applying fixes
7 months, 3 weeks ago (2016-11-30 01:24:47 UTC) #15
imysak
Just few words to describe my CL. In Adapters we have two pusher strategies: Full(non-incremental) ...
7 months, 2 weeks ago (2016-12-05 18:31:13 UTC) #16
pjo
Thank you https://codereview.appspot.com/311210043/diff/120001/src/com/google/enterprise/adaptor/DocIdSender.java File src/com/google/enterprise/adaptor/DocIdSender.java (right): https://codereview.appspot.com/311210043/diff/120001/src/com/google/enterprise/adaptor/DocIdSender.java#newcode226 src/com/google/enterprise/adaptor/DocIdSender.java:226: defs, caseSensitive, !journal.isFullPushStarted(), handler); Can both full ...
7 months, 2 weeks ago (2016-12-05 23:34:27 UTC) #17
imysak
https://codereview.appspot.com/311210043/diff/120001/src/com/google/enterprise/adaptor/DocIdSender.java File src/com/google/enterprise/adaptor/DocIdSender.java (right): https://codereview.appspot.com/311210043/diff/120001/src/com/google/enterprise/adaptor/DocIdSender.java#newcode226 src/com/google/enterprise/adaptor/DocIdSender.java:226: defs, caseSensitive, !journal.isFullPushStarted(), handler); On 2016/12/05 23:34:27, pjo wrote: ...
7 months, 2 weeks ago (2016-12-06 01:11:42 UTC) #18
imysak
I just make small debug to verify my bug or ensure in safety of my ...
7 months, 2 weeks ago (2016-12-06 16:11:37 UTC) #19
JohnL
Just stopping by for a peek. Overall I like this, and basically the OpenText and ...
7 months, 2 weeks ago (2016-12-07 02:16:39 UTC) #20
JohnL
4 months, 1 week ago (2017-03-16 22:32:40 UTC) #21

          
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 80a51fa-tainted