Trying again at using the python samba_kcc

[Andrew Bartlett]

On Wed, 2015-05-20 at 16:29 +1200, Douglas Bagnall wrote:
> hi Garming,
> 
> Thanks. I've addressed these things and pushed to 
> http://git.catalyst.net.nz/gitweb?p=samba.git;a=shortlog;h=refs/heads/kcc-intersite-27
> 
> On 15/05/15 22:27, garming at catalyst.net.nz wrote:
> > Hi,
> > 
> > So I've spent some time reviewing some of the patches. The last dozen or
> > so I've left off for now since Douglas hasn't signed off on them yet.
> > 
> > I've looked over the changes and they all seem fairly reasonable. I've
> > made my notes in the form of TODO on the tree.
> > 
> > Douglas:
> > - The edge list always being empty in your comment is slightly worrying
> 
> Yes, but fortunately I was wrong -- or at least I have since patched
> up and rebased away the thing that made me right.
> 
> > - There's a few commits that should probably be merged without too much
> > trouble
> 
> > - There is a commit that implies RO is partial when it may not be the case?
> 
> That was not about RO at all, but I can see how the comment didn't
> explain that well enough. If the patch had 60 lines of context it
> would be clear. I hope this message improves it:
> 
> http://git.catalyst.net.nz/gitweb?p=samba.git;a=commitdiff;h=1566d43ae
> 
> (vs the original, with Garming's query:
> https://git.samba.org/?p=garming/samba.git;a=commit;h=8e180eb7ec51 )
> 
> > - There's some code you highlighted that might be dealing with schedules
> > wrong - should it be fixed? And in general, what should None schedules
> > mean? I think you've noted that there are differences with it being always
> > in some places and being that 15 minute block every hour in others - could
> > it always mean the latter?
> 
> I have clarified the comment you highlighted. 
> 
> The source of the confusion is that there are two schedule types
> (ReplTimes and Schedule) used by different objects, each packaging the
> same information in different ways. In the specification, both types
> are called "schedule" in variables and attributes, and we followed the
> spec in our naming. In the original C++ these would have been
> disambiguated by the declared types, but in python it is unclear which
> schedule type is in use at any point. A null Schedule tends to mean
> "never" while a null ReplInfo tends to mean "always".
> 
> > The last things I think is that, I see you've removed the SMTP code among
> > other things. But you've also removed some of the failed link and site
> > link bridge code. For the stuff that could be implemented still and isn't
> > stubbed out anymore, it should probably be noted well somewhere. I still
> > think there might be some cases which are executed in the remove
> > unnecessary failed links but we sort of would need a local store before it
> > made more sense.
> 
> Right. I've reverted that one. I think this part:
> 
> -        # Remove all tuples in kcc_failed_links where failure count = 0
> -        # In this implementation, this should never happen.
> 
> really doesn't apply to us, but I haven't thought through the other bit.
> 
> > I've pushed it with my review to my public Samba git repo:
> > 
> > git://git.samba.org/garming/samba.git
> > https://git.samba.org/?p=garming/samba.git;a=shortlog;h=refs/heads/kcc-intersite-to-review
> 
> There is one TODO commit by abartlet at samba.org. Andrew, can you look
> at it?
> 
> http://git.catalyst.net.nz/gitweb?p=samba.git;a=commitdiff;h=ab587cc2f4ff

Yes, I'll fix that up. 

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba